-
Notifications
You must be signed in to change notification settings - Fork 66
Improve verifications to verify rollout deployments #321
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
Improve verifications to verify rollout deployments #321
Conversation
|
@olamy ping |
695feaa to
70109be
Compare
70109be to
9d124f6
Compare
9d124f6 to
4a6e3b0
Compare
With the exception of progress deadline, it now uses the same algorith as "kubectl rollout status": https://github.com/kubernetes/kubectl/blob/f89fc21e9c51d313e923eb93d1ae83754be62019/pkg/polymorphichelpers/rollout_status.go#L80-L88 While at it, rename minimum replicas to desired replicas as per Kubernetes terminology. Minimum replicas are more used in Horizontal Pod Autoscaling, not the Deployment configuration.
4a6e3b0 to
9167347
Compare
|
@basil can this be merged? The plugin seems to be abandoned by Google, but it is still useful. |
|
Hi @ViliusS, I'm not doing feature work on this plugin, but you can adopt it following the instructions at https://www.jenkins.io/doc/developer/plugin-governance/adopt-a-plugin/ |
|
@olamy @craigdbarber @stephenashank @donmccasland @batmat @raul-arabaolaza is anyone still maintaining this plugin, o was it abandoned? |
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.
Pull Request Overview
This PR improves the verification process for rollout deployments by aligning the logic with the kubectl rollout status algorithm. Key changes include updating test assertions to reflect new replica terminology, modifying the log message to use a pre-defined message constant, and revising the deployment verification logic to compare available, updated, and desired replicas.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/java/com/google/jenkins/plugins/k8sengine/KubernetesVerifiersTest.java | Updated test assertions and renamed variables to match the new verification logic |
| src/main/java/com/google/jenkins/plugins/k8sengine/VerificationTask.java | Updated the log message format to use a message constant for consistency |
| src/main/java/com/google/jenkins/plugins/k8sengine/KubernetesVerifiers.java | Revised the deployment verification logic and updated related JSON paths and constants |
Comments suppressed due to low confidence (1)
src/main/java/com/google/jenkins/plugins/k8sengine/KubernetesVerifiers.java:175
- Adding an inline comment to explain the rationale behind these specific replica comparisons (and how they mirror the kubectl rollout status behavior) would improve code clarity and maintainability.
&& availableReplicas.intValue() == updatedReplicas.intValue();
This PR uses the same algorithm as "kubectl rollout status" for the verification of rollout deployments.
Fixes #119