Upgrade uses new helm repo #1069
Conversation
There was a problem hiding this comment.
Looking 🔥 !
Excited about all the cleanup.
Some polish that would be really neat
- ask for
--profile-version=v0.0.14flag that we could set the helmrelease version to - Set the
values.config.cluster.namein the helm release tocnamevar. - Set the
values.config.capi.repositoryURLto --app-config-url
🤔
🙌
pkg/upgrade/upgrade.go
Outdated
| if uv.GitRepository == "" { | ||
| gitRepositoryNameNamespace := fmt.Sprintf("%s/%s", uv.Namespace, strings.TrimSuffix(filepath.Base(repoUrlString), ".git")) | ||
| fmt.Fprintf(w, "Deriving name of GitRepository Resource as %v\n", gitRepositoryNameNamespace) | ||
| uv.GitRepository = gitRepositoryNameNamespace | ||
| } |
There was a problem hiding this comment.
I think we can remove this now as it was only important for profiles.
- also rm the
ensureGitRepositoryResourceandgetGitRepositoryNamespaceAndNametoo
cmd/gitops/upgrade/cmd.go
Outdated
| providerClient := internal.NewGitProviderClient(os.Stdout, os.LookupEnv, auth.NewAuthCLIHandler, log) | ||
|
|
||
| gitClient, gitProvider, err := factory.GetGitClients(ctx, providerClient, services.GitConfigParams{ | ||
| URL: upgradeCmdFlags.RepoURL, |
There was a problem hiding this comment.
Nice, I think this is simpler to do it as a flag instead of reading from the gitconfig.
Maybe we rename this flag to --app-config-url to be consistent w/ gitops install --app-config-url.
And we can remove the code that tries to automatically figure this out ( repoUrlString, err := gitClient.GetRemoteUrl(".", uv.Remote)).
I don't think we need RepoOrgAndName anymore either so we can get rid of the whole buildUpgradeConfigs probably. 🤔
There was a problem hiding this comment.
Thank you very much Simon 🙏 I will add the suggest changes 👍
pkg/upgrade/upgrade.go
Outdated
| "repositoryURL": "%s" | ||
| } | ||
| } | ||
| }` |
There was a problem hiding this comment.
We can do a literal here to avoid the decode w/ something like this (made a little demo 🎉 )
There was a problem hiding this comment.
Thank you very much Simon, that looks much better than using strings too 👍. I tried to do it with nested map[string]interface{} as well but it doesn't look very readable 😅
There was a problem hiding this comment.
Looking great! So nice to have --set here. 🔥 well done :)
We had some feedback around flags. Could we remove all the ones we don't use anymore..
- Remote
- ConfigMap
- Out (?)
- Maybe others?
and make sure the other ones are the similar to add app / install as possible?
This switches from installing the weave-gitops-enterprise profile from a git-repository using pctl to installing from a helmrepository! Adds support for --dry-run Adds support for --set capi.cluster.name=foo Configures an interval for the helmrelease so changes are re-applied Auto set clusterName from env Auto set repoURL from --app-config-url
This switches from installing the weave-gitops-enterprise profile from a git-repository using
pctlto installing from a helmrepository!--dry-run--set capi.cluster.name=foo--app-config-url