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.

Sign up for GitHub

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

Jump to bottom

Store the hashed ApplicationSet Spec inside the ProgressiveSync state manager #135

Merged
merged 23 commits into from
Sep 20, 2021
Merged

Store the hashed ApplicationSet Spec inside the ProgressiveSync state manager #135

merged 23 commits into from
Sep 20, 2021

Conversation

DimitarHristov111
Copy link
Contributor

@DimitarHristov111 DimitarHristov111 commented Sep 8, 2021 •
edited

Background

This PR will contribute and help us toward achieving - #63

Changes

  • Fixed the build to correctly download the kubebuilder dep;
  • Updated the chart to allow us to get and list ApplicationSet objects;
  • Updated the controller to calculate a hash value from the ApplicationSet spec and store it in memory (for now);
  • Added tests;
  • Updated the state manager to allow us to store hash values in there;
  • Added github.com/argoproj-labs/applicationset as a dependency;
  • Added applicationset as part of the controller scheme;
  • Added the ApplicationSet CRD manifest;

Testing

  • Updated tests run as part of the project;
  • Tested with a local cluster and did a demo in front of the squad;

Outstanding tasks

  • Mirror the chart once the PR is merged
  • Deploy the new controller
  • Change the ProgressiveSync scheme to include the namespace as part of the SourceRef section of the manifest.

DimitarHristov111
DimitarHristov111 commented Sep 15, 2021
View reviewed changes
.github/workflows/test-build.yaml Outdated Show resolved Hide resolved
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
hack/argoproj.io_applicationsets.yaml
@@ -0,0 +1,6377 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Taken from https://github.com/argoproj-labs/applicationset/blob/v0.2.0/manifests/crds/argoproj.io_applicationsets.yaml as it is needed for our tests.

@DimitarHristov111 DimitarHristov111 changed the title Adding the hash value of the ApplicationSet Spec to the annotations of the ProgressiveSync resource Store the hashed ApplicationSet Spec inside the ProgressiveSync state manager Sep 15, 2021
maruina
maruina reviewed Sep 15, 2021
View reviewed changes
Copy link
Contributor

@maruina maruina left a comment

Choose a reason for hiding this comment

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

A couple of comments but generally looks good!

controllers/progressivesync_controller.go Show resolved Hide resolved
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
@DimitarHristov111 DimitarHristov111 mentioned this pull request Sep 15, 2021
Merged
@DimitarHristov111
Merge branch 'main' into applicationset-hash-annotation
5e2becd
@DimitarHristov111
Add a return if we fail to calculate the hash value; update tests
2d09720
@DimitarHristov111
Use argocd controls a Argo CD server
255e327 
Usage:
  argocd [flags]
  argocd [command]

Available Commands:
  account     Manage account settings
  app         Manage applications
  cert        Manage repository certificates and SSH known hosts entries
  cluster     Manage cluster credentials
  completion  output shell completion code for the specified shell (bash or zsh)
  context     Switch between contexts
  gpg         Manage GPG keys used for signature verification
  help        Help about any command
  login       Log in to Argo CD
  logout      Log out from Argo CD
  proj        Manage projects
  relogin     Refresh an expired authenticate token
  repo        Manage repository connection parameters
  repocreds   Manage repository connection parameters
  version     Print version information

Flags:
      --auth-token string               Authentication token
      --client-crt string               Client certificate file
      --client-crt-key string           Client certificate key file
      --config string                   Path to Argo CD config (default "/Users/dimitarhristov/.argocd/config")
      --grpc-web                        Enables gRPC-web protocol. Useful if Argo CD server is behind proxy which does not support HTTP2.
      --grpc-web-root-path string       Enables gRPC-web protocol. Useful if Argo CD server is behind proxy which does not support HTTP2. Set web root.
  -H, --header strings                  Sets additional header to all requests made by Argo CD CLI. (Can be repeated multiple times to add multiple headers, also supports comma separated headers)
  -h, --help                            help for argocd
      --insecure                        Skip server certificate and domain verification
      --logformat string                Set the logging format. One of: text|json (default "text")
      --loglevel string                 Set the logging level. One of: debug|info|warn|error (default "info")
      --plaintext                       Disable TLS
      --port-forward                    Connect to a random argocd-server port using port forwarding
      --port-forward-namespace string   Namespace name which should be used for port forwarding
      --server string                   Argo CD server address
      --server-crt string               Server certificate file

Use "argocd [command] --help" for more information about a command. as a parameter
@DimitarHristov111
Use ApplicationSet and change the way we calculate the hash value
637907b
@DimitarHristov111
Add missing comment
80bd256
maruina
maruina reviewed Sep 16, 2021
View reviewed changes
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
controllers/progressivesync_controller.go Outdated Show resolved Hide resolved
controllers/progressivesync_controller.go Show resolved Hide resolved
controllers/progressivesync_controller_test.go Outdated Show resolved Hide resolved
@DimitarHristov111 @maruina
Update controllers/progressivesync_controller_test.go
eb19ec0 
Co-authored-by: Matteo Ruina <matteo.ruina@gmail.com>
@DimitarHristov111 @maruina
Update controllers/progressivesync_controller.go
78f86c3 
Co-authored-by: Matteo Ruina <matteo.ruina@gmail.com>
@DimitarHristov111 @maruina
Update controllers/progressivesync_controller.go
a8b87c7 
Co-authored-by: Matteo Ruina <matteo.ruina@gmail.com>
@DimitarHristov111
Change the method name
124f63c
maruina
maruina approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@maruina maruina left a comment

Choose a reason for hiding this comment

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

LGTM

sledigabel
sledigabel approved these changes Sep 20, 2021
View reviewed changes
Copy link
Collaborator

@sledigabel sledigabel left a comment

Choose a reason for hiding this comment

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

LGTM, introduces a hash annotation that parses the AppSet and produces a unique hash, and detects a change on the deployment content like that.
The hash is stored in the in-memory store for now.

@DimitarHristov111 DimitarHristov111 merged commit 13445f3 into main Sep 20, 2021
@DimitarHristov111 DimitarHristov111 deleted the applicationset-hash-annotation branch September 20, 2021 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitar-hristov dimitar-hristov dimitar-hristov left review comments

@sledigabel sledigabel sledigabel approved these changes

@maruina maruina maruina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@DimitarHristov111 @sledigabel @maruina @dimitar-hristov