Added health check for pvc resource in argocd.yaml

[day0hero]

This allows argo to continue rolling out the rest of the applications whenever a PVC is in a pending status, which presents a chicken/egg scenario for a deployment/deploymentConfig depending on that PVC. The storageclass bindingmode is WaitForFirstConsumer in the hyperscalers, and to override this we would either need to make a separate storageclass with Immediate volumebinding and change the order of deployment.

With health check implemented, when the pvc is pending, bound it is considered healthy, and when it reports an error will show as unhealthy.

Without the health check the application is stuck in a progressing state and will not continue thus preventing any downstream application from deploying.

This directly affects devsecops because we create a pvc for when the pipeline runs, which is n minutes/hours/days after deployment. However, all other patterns are affected that create a PVC and Deployment/DeploymentConfig in separate manifests.

[mbaldessari]

Is it possible to limit this healthcheck override only when the storageclass is of a certain type? I.e. when the storageclass bindingmode is WaitForFirstConsumer in this case? Or do we not get that info in lua in the plugin?

If not maybe we should consider hiding this code behind an if and a helm variable or are we sure that we absolutely always want to do this all the time? (I genuinely have no idea about potentially unintended consequences, hence me being somewhat cautious)

[mbaldessari]

Also add a one-two line comment above the plugin so our future selves will remember why that thing is there ;)

[claudiol]

@day0hero I would make a slight modification in the health.lua code snippet:

health.lua: |
        hs = {}
        if obj.status ~= nil then
          if obj.status.phase ~= nil then
            if obj.status.phase == "Pending" then
              hs.status = "Healthy"
              hs.message = obj.status.phase
              return hs
            elseif obj.status.phase == "Bound" then
              hs.status = "Healthy"
              hs.message = obj.status.phase
              return hs
            end
          end
        end

Since you are checking the same variable the elseif allows you to execute one block of code if the specified condition is evaluates to "Pending" and another block of code if it is evaluates to "Bound". It feels more efficient.

[day0hero]

Is it possible to limit this healthcheck override only when the storageclass is of a certain type? I.e. when the storageclass bindingmode is WaitForFirstConsumer in this case? Or do we not get that info in lua in the plugin?

WaitForFirstConsumer is the default bindingMode in all of the hyperscalers - but to figure out if the storageclass is set that way would require a lookup/parsing of the storageclass.

If not maybe we should consider hiding this code behind an if and a helm variable or are we sure that we absolutely always want to do this all the time? (I genuinely have no idea about potentially unintended consequences, hence me being somewhat cautious)

I think that the pvc change could/should be universal since it does affect managing pvcs and deployment/deploymentconfigs as separated resource files (pvc.yaml, dc.yaml) .. I've worked around this previously by adding both resources in the same manifest (ex: deployment has pvc and deployment resources declared) ...

I can certainly add a conditional for this if we think its the best long term solution

[day0hero]

@day0hero I would make a slight modification in the health.lua code snippet:

health.lua: |
        hs = {}
        if obj.status ~= nil then
          if obj.status.phase ~= nil then
            if obj.status.phase == "Pending" then
              hs.status = "Healthy"
              hs.message = obj.status.phase
              return hs
            elseif obj.status.phase == "Bound" then
              hs.status = "Healthy"
              hs.message = obj.status.phase
              return hs
            end
          end
        end

Since you are checking the same variable the elseif allows you to execute one block of code if the specified condition is evaluates to "Pending" and another block of code if it is evaluates to "Bound". It feels more efficient.

Sure, I'll make the change and test.

[day0hero]

Tested the elseif change and it worked fine.

[mbaldessari]

Okay let's merge this then!