Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: add support for namespace other than flux-system #3532

Closed
wants to merge 7 commits into from

Conversation

waleedhammam
Copy link
Contributor

@waleedhammam waleedhammam commented Oct 22, 2023

Closes #3473

What changed?

  • add support for namespace other than flux-system

How did you validate the change?

  • Unit tests

How to test?

  • bootstrap flux in a namespace of your choice (test-namespace)
    flux bootstrap git \
      --url=ssh://git@<host>/<org>/<repository> \
      --branch=main \
      --private-key-file=<path/to/private.key> \
      --password=<key-passphrase> \
      --path=clusters/my-cluster
     -n test-namespace
  • start the CLI bootstrap process using this namespace gitops bootstrap -n test-namespace
  • this means every command runs from this CLI will address test-namespace namespace instead of flux-system
  • note: entitlement aswell will be in this namespace
  • note: starting without namespace will take flux-system by default

Release notes

Documentation Changes

Other follow ups

@waleedhammam waleedhammam added the enhancement New feature or request label Oct 22, 2023
Copy link
Contributor

@enekofb enekofb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

missing acceptance and testing myself under #3542

added also some other comments

@@ -52,7 +58,7 @@ func Command(opts *config.Options) *cobra.Command {
Example: cmdExamples,
RunE: getBootstrapCmdRun(opts),
}

cmd.PersistentFlags().StringVarP(&flags.namespace, "namespace", "n", wgeDefaultNamespace, "The namespace scope for this operation")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this flag different to

rootCmd.PersistentFlags().StringP("namespace", "n", defaultNamespace, "The namespace scope for this operation")
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm overriding it to be able to pass it to the commands
The root app doesn't refer it to a pointer or variable

@@ -59,6 +60,11 @@ func (c *ConfigBuilder) WithLogWriter(logger logger.Logger) *ConfigBuilder {
return c
}

func (c *ConfigBuilder) WithNamespace(namespace string) *ConfigBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is flux-system resolved if no namespace is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the flag itself, if it's not passed, then automatically set to flux system

@enekofb
Copy link
Contributor

enekofb commented Oct 25, 2023

when testing i get this error where i could see that the namespace has changed but i believe the kusktomization might be require also to change?

► verifying flux reconcillation in namespace: acceptance
    cmd_acceptance_test.go:148: 
        Expected
            <*errors.errorString | 0x140000d2fd0>: 
            cannot execute bootstrap: cannot execute 'checking flux': flux bootstrapped error: failed to reconcile flux source acceptance:flux-system: ✗ gitrepositories.source.toolkit.fluxcd.io "flux-system" not found
            . please bootstrap Flux: more info https://fluxcd.io/flux/installation
            {
                s: "cannot execute bootstrap: cannot execute 'checking flux': flux bootstrapped error: failed to reconcile flux source acceptance:flux-system: ✗ gitrepositories.source.toolkit.fluxcd.io \"flux-system\" not found\n. please bootstrap Flux: more info https://fluxcd.io/flux/installation",
            }
        to be nil
    cmd_acceptance_test.go:256: "level"=0 "msg"="deleted object" "name"="weave-gitops-enterprise-credentials" "ns"="acceptance" "kind"="Secret"

using the acceptance test aeb36f8#diff-82c4512096c6663a9b8f6901f0bd9c7e88088210551b054be8fbc3ca2ca0da1c

@waleedhammam waleedhammam marked this pull request as draft October 26, 2023 15:41
@waleedhammam
Copy link
Contributor Author

Closing as it's not doable to use another namespace as it's hard-coded in flux
PR became obsolete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bootstrap cli: allow dealing with flux namespaces otherthan flux system
2 participants