-
Notifications
You must be signed in to change notification settings - Fork 215
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
Wait for applied objects to become healthy using CEL readiness rule or kstatus #2419
Wait for applied objects to become healthy using CEL readiness rule or kstatus #2419
Conversation
390569d
to
0d2ecf9
Compare
- apply + wait for ready - Change Applier to get status using CEL rules if rule present - fallback Logic to get status of applied objects using kstatus - Applier.AreResourcesReady() wraps readiness checks - Each stage is applied and then checks if resources are ready (not blocking reconciler) - If resources are not ready, reconciler returns with an error causing a queued reconcile. - CEL based Readiness Rule support - Change Composition spec to take a list of CEL rules for custom status rules - pkg/cel/engine.go - Simple CEL wrapper to run rules against - Testcases: - TestFirstCompositionTeamPage: applies teampage sample (will tests waiting for deployment becoming healthy) - TestCustomStatusCELRule: tests using a CEL readiness rule for Deployment
@@ -16,6 +16,7 @@ apiVersion: composition.google.com/v1alpha1 | |||
kind: Composition | |||
metadata: | |||
name: team-page | |||
namespace: default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should start with the CC structure in which case this would be "config-control".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do. But this should work fine in any namespace. Compositions can exist in any namespace that the admin chooses it to be. The Facades can be in any namespace as well.
} else { | ||
// Fall back to kstatus | ||
result, err := status.Compute(u) | ||
if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe handle err != nil more clearly here? But I guess we're already handling it acceptably
|
||
func NewEngine(u *unstructured.Unstructured) (*Engine, error) { | ||
// TODO: what withtagname ? can we pass yaml ? | ||
registry := easycel.NewRegistry("cel-engine", easycel.WithTagName("json")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should ask the CEL folk what CEL library they recommend
Generally LGTM. A few comments, the only real blocker I think is whether we need to refetch the object when checking status. I don't think we do; we should have it from the apply. |
6403bfd
to
9197897
Compare
With the changes in KDP to allow callbacks for computing health, we no longer need to fetch the object. Updated the code (second commit) to register a callback and not fetch anymore. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
f5d86cc
into
GoogleCloudPlatform:master
Change description
a queued reconcile.
rules
deployment becoming healthy)
Deployment
Sample outputs
CEL rule based checks
CEL rule in TestCustomStatusCELRule/input.yaml:
readyIf: "status.readyReplicas==status.replicas"
plan status showing CEL rule result:
kstatus based checks
plan status showing waiting for resources to become healthy:
plan status with all resources healthy: