Skip to content

Commit bd5e34c

Browse files
deads2kdeads2k
committed
when updating status, do a live GET after conflict
1 parent 5674ec6 commit bd5e34c

File tree

8 files changed

+71
-12
lines changed

8 files changed

+71
-12
lines changed

‎pkg/operator/configobserver/config_observer_controller_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ func (c *fakeOperatorClient) GetOperatorState() (spec *operatorv1.OperatorSpec,
4343
return c.startingSpec, &operatorv1.OperatorStatus{}, "", nil
4444
}
4545

46+
func (c *fakeOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error) {
47+
return c.GetOperatorState()
48+
}
49+
4650
func (c *fakeOperatorClient) UpdateOperatorSpec(ctx context.Context, rv string, in *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
4751
if c.specUpdateFailure != nil {
4852
return &operatorv1.OperatorSpec{}, rv, c.specUpdateFailure

‎pkg/operator/genericoperatorclient/dynamic_operator_client.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,19 @@ func (c dynamicOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *op
8989
}
9090
instance := uncastInstance.(*unstructured.Unstructured)
9191

92+
return getOperatorStateFromInstance(instance)
93+
}
94+
95+
func (c dynamicOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
96+
instance, err := c.client.Get(ctx, c.configName, metav1.GetOptions{})
97+
if err != nil {
98+
return nil, nil, "", err
99+
}
100+
101+
return getOperatorStateFromInstance(instance)
102+
}
103+
104+
func getOperatorStateFromInstance(instance *unstructured.Unstructured) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
92105
spec, err := getOperatorSpecFromUnstructured(instance.UnstructuredContent())
93106
if err != nil {
94107
return nil, nil, "", err

‎pkg/operator/genericoperatorclient/dynamic_staticpod_operator_client.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorState() (*operatorv1
4848
}
4949
instance := uncastInstance.(*unstructured.Unstructured)
5050

51+
return getStaticPodOperatorStateFromInstance(instance)
52+
}
53+
54+
func getStaticPodOperatorStateFromInstance(instance *unstructured.Unstructured) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
5155
spec, err := getStaticPodOperatorSpecFromUnstructured(instance.UnstructuredContent())
5256
if err != nil {
5357
return nil, nil, "", err
@@ -66,16 +70,7 @@ func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx
6670
return nil, nil, "", err
6771
}
6872

69-
spec, err := getStaticPodOperatorSpecFromUnstructured(instance.UnstructuredContent())
70-
if err != nil {
71-
return nil, nil, "", err
72-
}
73-
status, err := getStaticPodOperatorStatusFromUnstructured(instance.UnstructuredContent())
74-
if err != nil {
75-
return nil, nil, "", err
76-
}
77-
78-
return spec, status, instance.GetResourceVersion(), nil
73+
return getStaticPodOperatorStateFromInstance(instance)
7974
}
8075

8176
func (c dynamicStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Context, resourceVersion string, spec *operatorv1.StaticPodOperatorSpec) (*operatorv1.StaticPodOperatorSpec, string, error) {

‎pkg/operator/managementstatecontroller/management_state_controller_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ func (c *statusClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1
135135
return &c.spec, &c.status, "", nil
136136
}
137137

138+
func (c *statusClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
139+
return c.GetOperatorState()
140+
}
141+
138142
func (c *statusClient) UpdateOperatorSpec(context.Context, string, *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
139143
panic("missing")
140144
}

‎pkg/operator/status/status_controller_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,10 @@ func (c *statusClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1
607607
return &c.spec, &c.status, "", nil
608608
}
609609

610+
func (c *statusClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
611+
return c.GetOperatorState()
612+
}
613+
610614
func (c *statusClient) UpdateOperatorSpec(context.Context, string, *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
611615
panic("missing")
612616
}

‎pkg/operator/v1helpers/helpers.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,21 @@ type UpdateStatusFunc func(status *operatorv1.OperatorStatus) error
159159
func UpdateStatus(ctx context.Context, client OperatorClient, updateFuncs ...UpdateStatusFunc) (*operatorv1.OperatorStatus, bool, error) {
160160
updated := false
161161
var updatedOperatorStatus *operatorv1.OperatorStatus
162+
numberOfAttempts := 0
162163
err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
163-
_, oldStatus, resourceVersion, err := client.GetOperatorState()
164+
defer func() {
165+
numberOfAttempts++
166+
}()
167+
var oldStatus *operatorv1.OperatorStatus
168+
var resourceVersion string
169+
var err error
170+
171+
if numberOfAttempts < 1 { // prefer lister if we haven't already failed.
172+
_, oldStatus, resourceVersion, err = client.GetOperatorState()
173+
174+
} else { // if we have failed enough times (chose 1 as a starting point, do a live GET
175+
_, oldStatus, resourceVersion, err = client.GetOperatorStateWithQuorum(ctx)
176+
}
164177
if err != nil {
165178
return err
166179
}
@@ -201,8 +214,21 @@ type UpdateStaticPodStatusFunc func(status *operatorv1.StaticPodOperatorStatus)
201214
func UpdateStaticPodStatus(ctx context.Context, client StaticPodOperatorClient, updateFuncs ...UpdateStaticPodStatusFunc) (*operatorv1.StaticPodOperatorStatus, bool, error) {
202215
updated := false
203216
var updatedOperatorStatus *operatorv1.StaticPodOperatorStatus
217+
numberOfAttempts := 0
204218
err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
205-
_, oldStatus, resourceVersion, err := client.GetStaticPodOperatorState()
219+
defer func() {
220+
numberOfAttempts++
221+
}()
222+
var oldStatus *operatorv1.StaticPodOperatorStatus
223+
var resourceVersion string
224+
var err error
225+
226+
if numberOfAttempts < 1 { // prefer lister if we haven't already failed.
227+
_, oldStatus, resourceVersion, err = client.GetStaticPodOperatorState()
228+
229+
} else { // if we have failed enough times (chose 1 as a starting point, do a live GET
230+
_, oldStatus, resourceVersion, err = client.GetStaticPodOperatorStateWithQuorum(ctx)
231+
}
206232
if err != nil {
207233
return err
208234
}

‎pkg/operator/v1helpers/interfaces.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ type OperatorClient interface {
1414
GetObjectMeta() (meta *metav1.ObjectMeta, err error)
1515
// GetOperatorState returns the operator spec, status and the resource version, potentially from a lister.
1616
GetOperatorState() (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error)
17+
// GetOperatorStateWithQuorum return the operator spec, status and resource version directly from a server read.
18+
GetOperatorStateWithQuorum(ctx context.Context) (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error)
1719
// UpdateOperatorSpec updates the spec of the operator, assuming the given resource version.
1820
UpdateOperatorSpec(ctx context.Context, oldResourceVersion string, in *operatorv1.OperatorSpec) (out *operatorv1.OperatorSpec, newResourceVersion string, err error)
1921
// UpdateOperatorStatus updates the status of the operator, assuming the given resource version.

‎pkg/operator/v1helpers/test_helpers.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ func (c *fakeStaticPodOperatorClient) GetStaticPodOperatorState() (*operatorv1.S
111111
return c.fakeStaticPodOperatorSpec, c.fakeStaticPodOperatorStatus, c.resourceVersion, nil
112112
}
113113

114+
func (c *fakeStaticPodOperatorClient) GetLiveStaticPodOperatorState() (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
115+
return c.GetStaticPodOperatorState()
116+
}
117+
114118
func (c *fakeStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx context.Context) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
115119
return c.fakeStaticPodOperatorSpec, c.fakeStaticPodOperatorStatus, c.resourceVersion, nil
116120
}
@@ -154,6 +158,9 @@ func (c *fakeStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Co
154158
func (c *fakeStaticPodOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
155159
return &c.fakeStaticPodOperatorSpec.OperatorSpec, &c.fakeStaticPodOperatorStatus.OperatorStatus, c.resourceVersion, nil
156160
}
161+
func (c *fakeStaticPodOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
162+
return c.GetOperatorState()
163+
}
157164
func (c *fakeStaticPodOperatorClient) UpdateOperatorSpec(ctx context.Context, s string, p *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
158165
panic("not supported")
159166
}
@@ -241,6 +248,10 @@ func (c *fakeOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *oper
241248
return c.fakeOperatorSpec, c.fakeOperatorStatus, c.resourceVersion, nil
242249
}
243250

251+
func (c *fakeOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
252+
return c.GetOperatorState()
253+
}
254+
244255
func (c *fakeOperatorClient) UpdateOperatorStatus(ctx context.Context, resourceVersion string, status *operatorv1.OperatorStatus) (*operatorv1.OperatorStatus, error) {
245256
if c.resourceVersion != resourceVersion {
246257
return nil, errors.NewConflict(schema.GroupResource{Group: operatorv1.GroupName, Resource: "TestOperatorConfig"}, "instance", fmt.Errorf("invalid resourceVersion"))

0 commit comments

Comments
 (0)