Set cluster Ready status only when all pods are ready#583
Set cluster Ready status only when all pods are ready#583andmat900 wants to merge 4 commits intoeiffel-community:mainfrom
Conversation
internal/controller/status/status.go
Outdated
| const ( | ||
| StatusAvailable = "Available" | ||
| StatusReady = "Ready" | ||
| StatusReconciling = "Reconciling" |
There was a problem hiding this comment.
Why add a new status for this? Can't we just set Ready=False with Reason=ReasonPending?
There was a problem hiding this comment.
This was suggested in the original issue: #406
There was a problem hiding this comment.
I disagree with myself from 1 year ago. I also do believe that it was wrong of me to give a suggested solution
andmat900
force-pushed
the
20260331_cluster_ready_status_check
branch
from
000d5b1 to
516a652
Compare
Change-Id: I786ce44952436ee245b71403bf20c5b13d690c95 Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
andmat900
force-pushed
the
20260331_cluster_ready_status_check
branch
from
516a652 to
337446d
Compare
Change-Id: I1a89fb69f012d6bfc8099787016f79df449d27ee Signed-off-by: Andrei Matveyeu <andrei.matveyeu@axis.com>
internal/etos/api/api.go
Outdated
| return err | ||
| } | ||
| return nil | ||
| return readiness.CheckDeployment(ctx, r.Client, namespacedName) |
There was a problem hiding this comment.
Why can't we just do these checks in, for example, reconcileDeployment? We already get the deployment there.
Change-Id: I1024b7aec42f3541bd2330955408b0159457b2d4 Signed-off-by: Andrei Matveyeu <andrei.matveyeu@axis.com>
Change-Id: I0f9abf8984b6d9f77445b82da3eb539466da1ea1 Signed-off-by: Andrei Matveyeu <andreimu@axis.com>
| if !readiness.IsNotReadyError(err) { | ||
| logger.Error(err, "Failed to reconcile the deployment for the ETOS API") | ||
| return err | ||
| } | ||
| notReadyErr = err |
There was a problem hiding this comment.
We should just return the error instead of moving all the way down the function and then return the error.
Is this because the deployment requires the other resources deployed, then we can just move the deployment reconciliation after those.
| return target, err | ||
| } | ||
| return target, nil | ||
| return target, readiness.DeploymentReady(target) |
There was a problem hiding this comment.
This should be an explicit NotReady return, since we have just created the deployment.
| return target, err | ||
| } | ||
| return target, r.Patch(ctx, target, client.StrategicMergeFrom(deployment)) | ||
| return target, readiness.DeploymentReady(target) |
There was a problem hiding this comment.
This should be an explicit NotReady return, since we have just patched the deployment.
|
|
||
| // DeploymentReady checks whether a Deployment has its desired number of ready replicas. | ||
| // Returns nil if ready, or a NotReadyError if not yet ready. | ||
| func DeploymentReady(dep *appsv1.Deployment) error { |
There was a problem hiding this comment.
Instead of dealing with counting replicas we should just use this condition and reason:
type: Progressing
status: "True"
reason: NewReplicaSetAvailable
https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#complete-deployment
2 participants
Applicable Issues
Fixes #406
Description of the Change
Add a readiness check after resource reconciliation that verifies all owned Deployments and StatefulSets have their desired ready replicas. A new Reconciling condition is set while pods are starting up, and Ready=True is only set once all pods are confirmed ready.
Alternate Designs
Considered label-based filtering instead of owner references, but owner references are already set by the controller and more reliable.
Possible Drawbacks
Sign-off
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
Signed-off-by: Andrei Matveyeu, andrei.matveyeu@axis.com