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

support CRDs created by controllers #2873

Closed
torstenwalter opened this issue Dec 13, 2019 · 9 comments · Fixed by #3247
Closed

support CRDs created by controllers #2873

torstenwalter opened this issue Dec 13, 2019 · 9 comments · Fixed by #3247
Labels
enhancement New feature or request

Comments

@torstenwalter
Copy link
Contributor

Summary

Argo CD should support deploying CRDs and manifests using the CRD instances in one commit even if the CRD manifest is not stored in version control, but another resource which takes care of creating the CRD.

Motivation

We are using Gatekeeper - Policy Controller for Kubernetes and want to deploy Constraint Templates and Constraints together.

I am quoting the examples from their documentation here:

apiVersion: templates.gatekeeper.sh/v1beta1
kind: ConstraintTemplate
metadata:
  name: k8srequiredlabels
spec:
  crd:
    spec:
      names:
        kind: K8sRequiredLabels
        listKind: K8sRequiredLabelsList
        plural: k8srequiredlabels
        singular: k8srequiredlabels
      validation:
        # Schema for the `parameters` field
        openAPIV3Schema:
          properties:
            labels:
              type: array
              items: string
  targets:
    - target: admission.k8s.gatekeeper.sh
      rego: |
        package k8srequiredlabels

        violation[{"msg": msg, "details": {"missing_labels": missing}}] {
          provided := {label | input.review.object.metadata.labels[label]}
          required := {label | label := input.parameters.labels[_]}
          missing := required - provided
          count(missing) > 0
          msg := sprintf("you must provide labels: %v", [missing])
        }
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sRequiredLabels
metadata:
  name: ns-must-have-gk
spec:
  match:
    kinds:
      - apiGroups: [""]
        kinds: ["Namespace"]
  parameters:
    labels: ["gatekeeper"]

If you commit both files together and Argo CD tries to sync them together then the sync status is the server could not find the requested resource.

The issue is that kind K8sRequiredLabels does not exist. There is also no CRD with that spec.
Actually that kind is created by the Gatekeeper controller which reacts to the ConstraintTemplate.

Of course one need to wait until the controller created that resource, but this would be possible by implementing a health check and putting the ConstraintTemplate to a sync wave before the Constraint.

Would be great if Argo CD could support this. Current workaround is to commit the ConstraintTemplate first, sync it with Argo CD and then create the Constaint.

Proposal

One option would be to allow to ignore non existing resources in dry-run. This could be enabled via a configuration parameter or be the default.

This is how it could look like:

if apierr.IsNotFound(err) && ignoreNonExistingResourcesInDryRun {
    ...
} else if apierr.IsNotFound(err) && sc.hasCRDOfGroupKind(task.group(), task.kind()) {
    sc.log.WithFields(log.Fields{"task": task}).Debug("skip dry-run for custom resource")
    task.skipDryRun = true
}

https://github.com/argoproj/argo-cd/blob/master/controller/sync.go#L475-L477

@torstenwalter torstenwalter added the enhancement New feature or request label Dec 13, 2019
@saviogl
Copy link

saviogl commented Dec 23, 2019

@torstenwalter I just came across this issue now deploying gatekeeper through ArgoCD. Do you have an interim approach for handling that while this proposal is in flight?

@torstenwalter
Copy link
Contributor Author

@saviogl The only workaround I see is to either deploy ConstraintTeamplate and Constraint seperately (different commits, wait until the first one is synced) or manually sync the ConstraintTeamplate...

For anything better a patch to Argo CD would be needed I guess.

@saviogl
Copy link

saviogl commented Dec 24, 2019

@torstenwalter Yeah, that's pretty much what I ended up having to do indeed. Have you heard back through other channels from the maintainers around this enhancement submission?

@OmerKahani
Copy link
Contributor

@saviogl The only workaround I see is to either deploy ConstraintTeamplate and Constraint seperately (different commits, wait until the first one is synced) or manually sync the ConstraintTeamplate...

could adding wave to the template can help?

@torstenwalter
Copy link
Contributor Author

@saviogl The only workaround I see is to either deploy ConstraintTeamplate and Constraint seperately (different commits, wait until the first one is synced) or manually sync the ConstraintTeamplate...

could adding wave to the template can help?

I tried this. Same error. Argo CD claims that the sync task is invalid.

@torstenwalter
Copy link
Contributor Author

@torstenwalter Yeah, that's pretty much what I ended up having to do indeed. Have you heard back through other channels from the maintainers around this enhancement submission?

No.

@OmerKahani
Copy link
Contributor

@saviogl The only workaround I see is to either deploy ConstraintTeamplate and Constraint seperately (different commits, wait until the first one is synced) or manually sync the ConstraintTeamplate...

could adding wave to the template can help?

I tried this. Same error. Argo CD claims that the sync task is invalid.

I think that the fix should be that the sync check each wve separately - this way it will be the same as two commits.

@torstenwalter
Copy link
Contributor Author

That makes sense, but then the check needs to happen before each wave to be able to take resources into account which have been created by a previous wave.

A more simplistic approach could be to have an annotation on the resource to tell Argo CD that it should not fail if the resource type does not exist.

@a-hat
Copy link
Contributor

a-hat commented Mar 17, 2020

To support CRDs created by k8s controller in one commit, we would like to introduce the following enhancement to ArgoCD:

  • Add a new option to the annotation argocd.argoproj.io/sync-options
  • The option will be named SkipDryRunOnMissingResource, and can be used on a resource in the following way: argocd.argoproj.io/sync-options: SkipDryRunOnMissingResource=true
  • When SkipDryRunOnMissingResource is set, then the dry run for this resource will be skipped when syncing, but only if the Group and Kind are currently not present in the cluster.
  • This is analogous to the current handling of missing CRDs.

Any feedback is appreciated.

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 a pull request may close this issue.

4 participants