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

Shared custom health check for multiple CRDs #4212

Closed
AmitBenAmi opened this issue Aug 31, 2020 · 15 comments
Closed

Shared custom health check for multiple CRDs #4212

AmitBenAmi opened this issue Aug 31, 2020 · 15 comments
Labels
enhancement New feature or request

Comments

@AmitBenAmi
Copy link

Summary

Instead of re-writing the same custom health check for different CRDs, share them altogether at once, under the same key in the ConfigMap.

Motivation

I have multiple CRDs under the same group and version, and all of them uses the same custom health check, that checks whether the status.synced == true and wrapping it with nil checks.
Each time I create another CRD, I need to copy/paste the custom health checks in ArgoCD's repository and deploy the ConfigMap, which causing a lot of overhead.

Proposal

I thought about 2 options:

  1. Instead of duplicating this check and writing another duplicated check whenever I create another CRD, it might be easier to maybe use wildcards to share the same check between these CRDs.
    For example: If the group name is foo.bar.com, and the resources are: foo and bar, then instead of foo.bar.com/foo: and foo.bar.com/bar: keys under resource.customizations in the ConfigMap, the key will be: foo.bar.com/*: (uses wildcard to capture all CRDs under this group).
  2. Let ArgoCD fetch custom health checks from different ConfigMaps.
    For instance, each CRDs sits in its own repository, which is different than ArgoCD's repository. When creating a CRD, it will create a ConfigMap for ArgoCD that configures all of the custom health checks. This way, ArgoCD will be able to fetch extra custom health checks from different ConfigMaps, but the CRD's owner will handle the checks in its corresponding repository.
@AmitBenAmi AmitBenAmi added the enhancement New feature or request label Aug 31, 2020
@jessesuen
Copy link
Member

Looks like we support this for diffing customization so it makes sense to do for health checks.

@AmitBenAmi
Copy link
Author

Is there any update regarding this issue?

@indu-subbaraj
Copy link

This would be really useful as I'm trying to add health checks for Gatekeeper Constraints , each of which is a unique CRD kind.

@hannesholst
Copy link

hannesholst commented May 10, 2021

Any updates on this request? It would be very beneficial for Google ConfigConnector resources as well.
They all have their own group and kind, it requires a lot of copy/pasting now.

In the mean time I did find a hacky workaround though, using a reference anchor to define the lua script once and reference it multiple times at the bottom for other CRDs 😄

With these you can list all CRDs available (name + kind) and with a Google Sheet parse it out a bit like the bottom lines:

kubectl get crds --selector cnrm.cloud.google.com/managed-by-kcc=true -o=custom-columns=GROUP:.spec.group
kubectl get crds --selector cnrm.cloud.google.com/managed-by-kcc=true -o=custom-columns=KIND:.spec.names.kind

resource.customizations: |
      pubsub.cnrm.cloud.google.com/PubSubSubscription: &id001
        health.lua: |
          hs = {}
          if obj.status ~= nil then
            if obj.status.conditions ~= nil then
              for i, condition in ipairs(obj.status.conditions) do
                if condition.type == "Ready" and condition.status == "False" then
                  hs.status = "Degraded"
                  hs.message = condition.message
                  return hs
                end
                if condition.type == "Ready" and condition.status == "True" then
                  hs.status = "Healthy"
                  hs.message = condition.message
                  return hs
                end
              end
            end
          end

          hs.status = "Progressing"
          hs.message = "Waiting for operator"
          return hs
      pubsub.cnrm.cloud.google.com/PubSubTopic: *id001
      iam.cnrm.cloud.google.com/IAMPolicyMember: *id001

@haarchri
Copy link

any updates here ? - in crossplane eco-system we also very interested in wildcards/shared custom health check crossplane/crossplane#2444

@reggie-k
Copy link
Member

We are also interested in this feature

@ItayZviCohen
Copy link

Would like this feature as well!
Use case: Health checks for Crossplane managed resources from same provider.

@reggie-k
Copy link
Member

For this

Summary

Instead of re-writing the same custom health check for different CRDs, share them altogether at once, under the same key in the ConfigMap.

Motivation

I have multiple CRDs under the same group and version, and all of them uses the same custom health check, that checks whether the status.synced == true and wrapping it with nil checks. Each time I create another CRD, I need to copy/paste the custom health checks in ArgoCD's repository and deploy the ConfigMap, which causing a lot of overhead.

Proposal

I thought about 2 options:

  1. Instead of duplicating this check and writing another duplicated check whenever I create another CRD, it might be easier to maybe use wildcards to share the same check between these CRDs.
    For example: If the group name is foo.bar.com, and the resources are: foo and bar, then instead of foo.bar.com/foo: and foo.bar.com/bar: keys under resource.customizations in the ConfigMap, the key will be: foo.bar.com/*: (uses wildcard to capture all CRDs under this group).
  2. Let ArgoCD fetch custom health checks from different ConfigMaps.
    For instance, each CRDs sits in its own repository, which is different than ArgoCD's repository. When creating a CRD, it will create a ConfigMap for ArgoCD that configures all of the custom health checks. This way, ArgoCD will be able to fetch extra custom health checks from different ConfigMaps, but the CRD's owner will handle the checks in its corresponding repository.

@jessesuen @crenshaw-dev
What path of the two ones @AmitBenAmi had suggested shall we take?

@reggie-k
Copy link
Member

I started implementing this for the main config map , with only the Kind being wildcard-able.

But actually it looks like for both Crossplane and GoogleConfigConnector the API Group also needs to be wildcard-able (for supporting pubsub.cnrm.cloud.google.com/PubSubTopic, iam.cnrm.cloud.google.com/IAMPolicyMember, ec2.aws.crossplane.io/Instance, iam.aws.crossplane.io/InstanceProfile, etc)

@reggie-k
Copy link
Member

reggie-k commented Oct 10, 2022

#10885
Implemented with the wildcard on Resource Kind level and updated docs.
Meaning, now :

      pubsub.cnrm.cloud.google.com/*:
        health.lua: |
           ...

can be configured.
However, iam.cnrm.cloud.google.com/*, for example, will still require a separate entry.
Is this good enough or is a wildcard on a Resource Group level needed as well?
Can something like

      *.cnrm.cloud.google.com/*:
        health.lua: |
           ...

be useful or unneeded? (regex matching potential performance implications)

@yogeek
Copy link

yogeek commented Oct 10, 2022

@reggie-k thank you for this !
We use crossplane and for us API Group also needs to be wildcard-able.
Indeed, currently, we have a script that generates the argocd-cm ConfigMap lua content by requesting the cluster to find every crossplane providers CRDs and adding each health check section from a template. And the result is
:

  • a lot of duplication on the resulting ConfigMap
  • the need to relaunch the script every time we add a provider

So covering a more generic use case with wild cards at different levels would be nice.

@reggie-k
Copy link
Member

Ok, so basically you (and me, as I also use Crossplane :)) want all of the following examples to be configurable for the health check:

      *.aws.crossplane.io/*:
        health.lua: |
           ...
      *.*.jet.crossplane.io/*:
        health.lua: |
           ...
      *.*.*.crossplane.io/*:
        health.lua: |
           ...
      *.*.crossplane.io/*:
        health.lua: |
           ...

In other words:
@yogeek @AmitBenAmi Each sub package of the API Group can be substituted with a * wildcard + Kind can be substituted with a * wildcard.
Does this cover the requirement?
@jessesuen @crenshaw-dev Is this design acceptable? Also, maybe for future versions we should consider adding another field in the config map allowing the users to specify a regex for the group and kind.

@yogeek
Copy link

yogeek commented Oct 11, 2022

@reggie-k that seems right 👍

crenshaw-dev added a commit that referenced this issue Nov 8, 2022
* Kind wildcard support in health customizations

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Document resource kind wildcard for custom health check

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Removed code duplication and returned an empty string instead of an error

Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this issue Nov 23, 2022
… (argoproj#10885)

* Kind wildcard support in health customizations

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Document resource kind wildcard for custom health check

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Removed code duplication and returned an empty string instead of an error

Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Jan 11, 2023
Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Jan 11, 2023
Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
emirot pushed a commit to emirot/argo-cd that referenced this issue Jan 27, 2023
… (argoproj#10885)

* Kind wildcard support in health customizations

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Updated health customizations docs to using the correct field with a /

Signed-off-by: reggie <reginakagan@gmail.com>

* Document resource kind wildcard for custom health check

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Implemented wildcard * support in API Group and Resource Kind and updated docs

Signed-off-by: reggie <reginakagan@gmail.com>

* Removed code duplication and returned an empty string instead of an error

Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
emirot pushed a commit to emirot/argo-cd that referenced this issue Jan 27, 2023
Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
schakrad pushed a commit to schakrad/argo-cd that referenced this issue Mar 14, 2023
Signed-off-by: reggie <reginakagan@gmail.com>

Signed-off-by: reggie <reginakagan@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: schakrad <chakradari.sindhu@gmail.com>
@brandocomando
Copy link

Any update here?

@crenshaw-dev
Copy link
Member

I believe this was implemented by #10885. If I'm wrong, lmk, and I will reopen!

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

No branches or pull requests

10 participants