Print out GitSource from config repository on dry-run #1320
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
Added function to convert from Manifest to CommitFile
Removed AssociateCluster method from GitOpsDirectoryWriter
# Conflicts: # pkg/services/gitops/install.go # pkg/services/gitops/install_test.go
# Conflicts: # pkg/services/automation/automation_test.go # pkg/services/automation/cluster_generator.go # pkg/upgrade/upgrade.go
# Conflicts: # cmd/gitops/install/cmd.go # pkg/models/manifest.go # pkg/models/manifest_test.go # pkg/services/auth/auth_test.go # pkg/services/gitopswriter/gitopswriter.go # pkg/services/install/install.go # pkg/services/install/install_test.go
# Conflicts: # pkg/services/gitops/uninstall.go
josecordaz
changed the title
jpellizzari
reviewed
Jan 26, 2022
| // This ensures that git operations are done with stored deploy keys instead of a user's local ssh-agent or equivalent. | ||
| func (a *authSvc) setupDeployKey(ctx context.Context, name SecretName, targetName string, repo gitproviders.RepoURL) (*ssh.PublicKeys, error) { | ||
| func (a *authSvc) SetupDeployKey(ctx context.Context, namespace string, repo gitproviders.RepoURL) (*ssh.PublicKeys, error) { |
There was a problem hiding this comment.
Wasn't this changed in another branch? I have the feeling that some stuff from other branches made it onto this PR.
There was a problem hiding this comment.
I can't tell if there are any other branches with this change. I intentionally changed this 🙂
# Conflicts: # cmd/gitops/install/cmd.go # pkg/services/factory.go
jpellizzari
approved these changes
Jan 26, 2022
There was a problem hiding this comment.
Having a tough time understanding exactly what is happening here, but I assume this was cleanup from the previous refactoring PR @josecordaz ?
Code LGTM
pkg/models/manifest.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| sourceManifest, err := GetSourceManifest(ctx, fluxClient, gitProvider, clusterName, namespace, configURL, configBranch) |
There was a problem hiding this comment.
Does this mean that this manifest isn't being applied on main?
There was a problem hiding this comment.
it is applied but not by using this function (which is the ideal scenario). That is why I wanted to create this PR.
josecordaz
added
bug
# Conflicts: # cmd/gitops/install/cmd.go # pkg/models/manifest.go # pkg/models/manifest_test.go # pkg/services/install/install.go # pkg/services/install/install_test.go
# Conflicts: # cmd/gitops/install/cmd.go # pkg/models/manifest_test.go # pkg/services/auth/auth.go # pkg/services/auth/auth_test.go # pkg/services/factory.go
jpellizzari
added a commit
that referenced
this pull request
Mar 3, 2022
* Make dry-run in install working * write manifests to repo after applying them * Moving gitOpsWriter out of Install * Run raw flux install to get fancy progress logging * Removed creation of namespace as `flux install` already creates it * Removed unused osysClient parameter * Create namespace before creating git clients * Created new repoWriter layer to simplify the code Added function to convert from Manifest to CommitFile * Moving generation of manifests files to models folder. Removed AssociateCluster method from GitOpsDirectoryWriter * Pack all constants in the same place * Removed cluster_generator.go file * Removed unused constants * Added error paths unit tests for installer * Added missing files changes * Fixed lint issues * Added unit tests manifests functions * Added initial code for install success when auto-merge is true * Added rest of code for install success when auto-merge is true * Fix lint issues * Removed old install tests * Remove unused variables * Added success path for install when auto-merge is false * Added missing validate wego installation functionality * Use the right yaml library * Fixed yaml dependency * Added missing keep and source manifests and fixed unit tests * Fixed manifests order in unit tests * Fixed acceptance test "Verify that gitops quits if flux-system namespace is present" * Fixed `install --dry-run` it was printing out raw bytes instead of string Fixed regex on acceptance test * Fixed Kustomize file on unit test * Fixed Kustomize file on unit test * Fixed typo and added specif error matches * Clean up and fixed typo on assertion message * Made some lines more readable * Rename function CreateKustomize to CreateKustomization Co-authored-by: Jordan Pellizzari <jordan@weave.works> * Renamed function from GitopsConfigMap to CreateGitopsConfigMap * Fixed references to old function A function was renamed but their calls were not * Fixed references to old function A function was renamed but their calls were not * Initial work to get token and deploy key right before setting git clients This work could let us use flags to pass in some info like 1.- Secret names(in case they are already in the cluster) 2.- Deploy keys path. In case the user has the deploy key saved in a file 3.- Provider deploy key name. In case the user already has a deploy key in the provider. That we can later save to the cluster as secret. We would need to authenticate the first time to retrieve it. Instantiating git clients separately * Added source flux resource back to dry-run output * Fixed lint issues * Fixed references to old function A function was renamed but their calls were not * Use the right manifests when creating the PR * Fixed unit test it was not generating adding the wego app path to the kustomization of the expected file * Clean up comments * Updated comments * Fixed installer unit tests * Fixed manifests unit tests * Removed commented code * Removed old SetupDeployKey2 function * Fixed lint issue * Fixed install unit tests * Fixed manifests unit tests * Fixed auth unit tests * Removed targetName field as it wasn't used * Fixed auth unit tests * Organize imports on install command * Fixed parameters on install/cmd.go Co-authored-by: Jordan Pellizzari <jordan@weave.works>
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:
What changed?
Split up git client instantiation on install
Removed old
targetNamefield fromgetGitClientsfunction as it wasn't usedMoved
GitSourcecreation tomodels.BootstrapManifestsas this is the function used in dry-run.Why?
To be able to print out GitSource on dry-run without setting up a deploy key, which is not needed for dry-run.
How did you test it?
Manually, CI
Release notes
Documentation Changes