Skip to content

Commit 2e340f0

Browse files
committed
controllers: Prevent progressing on scaling only
Assisted-by: Claude Code
1 parent 7156db3 commit 2e340f0

5 files changed

Lines changed: 712 additions & 30 deletions

File tree

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package scaling
2+
3+
import (
4+
"fmt"
5+
"time"
6+
7+
appsv1 "k8s.io/api/apps/v1"
8+
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/apimachinery/pkg/api/equality"
10+
"k8s.io/utils/clock"
11+
"k8s.io/utils/ptr"
12+
13+
operatorv1 "github.com/openshift/api/operator/v1"
14+
applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
15+
)
16+
17+
const (
18+
replicasChangedAnnotation = "authentication.operator.openshift.io/replicas-changed"
19+
deploymentProgressedAnnotation = "authentication.operator.openshift.io/deployment-progressed"
20+
scalingBeginTimeout = 1 * time.Minute
21+
)
22+
23+
// ProcessDeployment ensures the operator does not end up progressing on scaling.
24+
// We define that scaling happens any time .spec.replicas is the only field that changes.
25+
// The idea is then as follows:
26+
//
27+
// 1. When the replicas field is updated, store the change timestamp in a deployment annotation.
28+
// 2. When the deployment eventually starts progressing, add another annotation so that we know it happened.
29+
// 3. When the deployment hasn't progressing for too long, or it has finished progressing, remove all annotations.
30+
//
31+
// When the timestamp annotation is present, we should overwrite Progressing to be false.
32+
//
33+
// So, ProcessDeployment amends the expected deployment in place, also returning any conditions to set on the operator.
34+
func ProcessDeployment(existing, expected *appsv1.Deployment, clock clock.Clock, conditionPrefix string) ([]*applyoperatorv1.OperatorConditionApplyConfiguration, error) {
35+
if !specsEqualIgnoringReplicas(existing, expected) {
36+
return nil, nil
37+
}
38+
39+
if expected.Annotations == nil {
40+
expected.Annotations = make(map[string]string)
41+
}
42+
43+
if !ptr.Equal(existing.Spec.Replicas, expected.Spec.Replicas) {
44+
expected.Annotations[replicasChangedAnnotation] = clock.Now().UTC().Format(time.RFC3339)
45+
return cancelProgressing(conditionPrefix), nil
46+
}
47+
48+
var replicasChangedAt time.Time
49+
if v, ok := existing.Annotations[replicasChangedAnnotation]; ok {
50+
var err error
51+
replicasChangedAt, err = time.Parse(time.RFC3339, v)
52+
if err != nil {
53+
return nil, fmt.Errorf("unable to parse annotation %q = %q: %w", replicasChangedAnnotation, v, err)
54+
}
55+
}
56+
if replicasChangedAt.IsZero() {
57+
return nil, nil
58+
}
59+
60+
// Cancel scaling if we are done, or the whole process has reached the specified timeout.
61+
startedProgressing := existing.Annotations[deploymentProgressedAnnotation] == "true"
62+
if !isDeploymentProgressing(existing.Status) && (startedProgressing || clock.Since(replicasChangedAt) > scalingBeginTimeout) {
63+
return nil, nil
64+
}
65+
66+
expected.Annotations[replicasChangedAnnotation] = existing.Annotations[replicasChangedAnnotation]
67+
if startedProgressing || isDeploymentProgressing(existing.Status) {
68+
expected.Annotations[deploymentProgressedAnnotation] = "true"
69+
}
70+
return cancelProgressing(conditionPrefix), nil
71+
}
72+
73+
// specsEqualIgnoringReplicas returns true when the deployment specs are the same or diff only in the replicas field.
74+
// The function returns false automatically when one of the deployments is nil.
75+
func specsEqualIgnoringReplicas(existing, expected *appsv1.Deployment) bool {
76+
if existing == nil || expected == nil {
77+
return false
78+
}
79+
80+
s1 := &existing.Spec
81+
s2 := &expected.Spec
82+
83+
if !ptr.Equal(s1.Replicas, s2.Replicas) {
84+
s2 = s2.DeepCopy()
85+
s2.Replicas = s1.Replicas
86+
}
87+
return equality.Semantic.DeepEqual(s1, s2)
88+
}
89+
90+
// isDeploymentProgressing returns whether the given deployment is progressing.
91+
func isDeploymentProgressing(status appsv1.DeploymentStatus) bool {
92+
for _, cond := range status.Conditions {
93+
if cond.Type == appsv1.DeploymentProgressing {
94+
return !(cond.Status == corev1.ConditionTrue && cond.Reason == "NewReplicaSetAvailable")
95+
}
96+
}
97+
return false
98+
}
99+
100+
func cancelProgressing(conditionPrefix string) []*applyoperatorv1.OperatorConditionApplyConfiguration {
101+
return []*applyoperatorv1.OperatorConditionApplyConfiguration{
102+
applyoperatorv1.OperatorCondition().
103+
WithType(fmt.Sprintf("%sDeploymentProgressing", conditionPrefix)).
104+
WithStatus(operatorv1.ConditionFalse).
105+
WithReason("AsExpected").
106+
WithMessage("Scaling replicas only"),
107+
}
108+
}

0 commit comments

Comments
 (0)