Connect cluster cmd #3239
Merged
Connect cluster cmd #3239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ranatrk
force-pushed
the
connect-cluster-cmd
branch
3 times, most recently
from
2f64295 to
b57cde9
Compare
ranatrk
force-pushed
the
connect-cluster-cmd
branch
from
b57cde9 to
3c5b17d
Compare
bigkevmcd
reviewed
Aug 25, 2023
| } | ||
|
|
||
| logger := stdr.New(nil) | ||
| ctx := log.IntoContext(context.Background(), logger) |
There was a problem hiding this comment.
This could probably call cmd.Context() rather than creating a new Background context?
ranatrk
force-pushed
the
connect-cluster-cmd
branch
4 times, most recently
from
1f14567 to
82c8f99
Compare
bigkevmcd
reviewed
Aug 31, 2023
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ServiceAccountName, "service-account", "weave-gitops-enterprise", "Service account name to be created/used") | ||
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ClusterRoleName, "cluster-role", "weave-gitops-enterprise", "Cluster role name to be created/used") | ||
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ClusterRoleBindingName, "cluster-role-binding", "weave-gitops-enterprise", "Cluster role binding name to be created/used") | ||
| cmd.Flags().StringVar(&connectOptionsCmdFlags.Namespace, "namespace", "default", "namespace of remote cluster") |
There was a problem hiding this comment.
We could change this to a StringVarP https://pkg.go.dev/github.com/spf13/pflag#StringToStringVarP and add a "short name" of n which would allow it to be used as -n for the namespace which has the same semantic as kubectl.
|
I also think we need to remove the two different ways of loading the config, but I've covered that elsewhere 😄 |
ranatrk
force-pushed
the
connect-cluster-cmd
branch
4 times, most recently
from
c7d7797 to
ba3ae07
Compare
bigkevmcd
approved these changes
Sep 4, 2023
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ServiceAccountName, "service-account", "weave-gitops-enterprise", "Service account name to be created/used") | ||
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ClusterRoleName, "cluster-role", "weave-gitops-enterprise", "Cluster role name to be created/used") | ||
| cmd.Flags().StringVar(&connectOptionsCmdFlags.ClusterRoleBindingName, "cluster-role-binding", "weave-gitops-enterprise", "Cluster role binding name to be created/used") | ||
| cmd.Flags().StringVarP(&connectOptionsCmdFlags.Namespace, "namespace", "n", "default", "namespace of remote cluster") |
There was a problem hiding this comment.
Let's make this Namespace of remote cluster in keeping with the other descriptions?
…ng two config loading methods
Co-authored-by: Kevin McDermott <[email protected]>
Co-authored-by: Kevin McDermott <[email protected]>
ranatrk
force-pushed
the
connect-cluster-cmd
branch
from
499d05e to
b927471
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3086
What changed?
Add
connect clustercmd to gitopsWhy was this change made?
to use connector pkg to connect a remote cluster to a cluster
How was this change implemented?
How did you validate the change?
Once building the gitops cmd with the updates:
Release notes
Documentation Changes
Other follow ups