Skip to content
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

when updating status, do a live GET after conflict #1658

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ func (c *fakeOperatorClient) GetOperatorState() (spec *operatorv1.OperatorSpec,
return c.startingSpec, &operatorv1.OperatorStatus{}, "", nil
}

func (c *fakeOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error) {
return c.GetOperatorState()
}

func (c *fakeOperatorClient) UpdateOperatorSpec(ctx context.Context, rv string, in *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
if c.specUpdateFailure != nil {
return &operatorv1.OperatorSpec{}, rv, c.specUpdateFailure
Expand Down
13 changes: 13 additions & 0 deletions pkg/operator/genericoperatorclient/dynamic_operator_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ func (c dynamicOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *op
}
instance := uncastInstance.(*unstructured.Unstructured)

return getOperatorStateFromInstance(instance)
}

func (c dynamicOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
instance, err := c.client.Get(ctx, c.configName, metav1.GetOptions{})
if err != nil {
return nil, nil, "", err
}

return getOperatorStateFromInstance(instance)
}

func getOperatorStateFromInstance(instance *unstructured.Unstructured) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
spec, err := getOperatorSpecFromUnstructured(instance.UnstructuredContent())
if err != nil {
return nil, nil, "", err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorState() (*operatorv1
}
instance := uncastInstance.(*unstructured.Unstructured)

return getStaticPodOperatorStateFromInstance(instance)
}

func getStaticPodOperatorStateFromInstance(instance *unstructured.Unstructured) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
spec, err := getStaticPodOperatorSpecFromUnstructured(instance.UnstructuredContent())
if err != nil {
return nil, nil, "", err
Expand All @@ -66,16 +70,7 @@ func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx
return nil, nil, "", err
}

spec, err := getStaticPodOperatorSpecFromUnstructured(instance.UnstructuredContent())
if err != nil {
return nil, nil, "", err
}
status, err := getStaticPodOperatorStatusFromUnstructured(instance.UnstructuredContent())
if err != nil {
return nil, nil, "", err
}

return spec, status, instance.GetResourceVersion(), nil
return getStaticPodOperatorStateFromInstance(instance)
}

func (c dynamicStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Context, resourceVersion string, spec *operatorv1.StaticPodOperatorSpec) (*operatorv1.StaticPodOperatorSpec, string, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ func (c *statusClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1
return &c.spec, &c.status, "", nil
}

func (c *statusClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
return c.GetOperatorState()
}

func (c *statusClient) UpdateOperatorSpec(context.Context, string, *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
panic("missing")
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/operator/status/status_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,10 @@ func (c *statusClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1
return &c.spec, &c.status, "", nil
}

func (c *statusClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
return c.GetOperatorState()
}

func (c *statusClient) UpdateOperatorSpec(context.Context, string, *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
panic("missing")
}
Expand Down
30 changes: 28 additions & 2 deletions pkg/operator/v1helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,21 @@ type UpdateStatusFunc func(status *operatorv1.OperatorStatus) error
func UpdateStatus(ctx context.Context, client OperatorClient, updateFuncs ...UpdateStatusFunc) (*operatorv1.OperatorStatus, bool, error) {
updated := false
var updatedOperatorStatus *operatorv1.OperatorStatus
numberOfAttempts := 0
err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
_, oldStatus, resourceVersion, err := client.GetOperatorState()
defer func() {
numberOfAttempts++
}()
var oldStatus *operatorv1.OperatorStatus
var resourceVersion string
var err error

if numberOfAttempts < 1 { // prefer lister if we haven't already failed.
_, oldStatus, resourceVersion, err = client.GetOperatorState()

} else { // if we have failed enough times (chose 1 as a starting point, do a live GET
_, oldStatus, resourceVersion, err = client.GetOperatorStateWithQuorum(ctx)
}
if err != nil {
return err
}
Expand Down Expand Up @@ -201,8 +214,21 @@ type UpdateStaticPodStatusFunc func(status *operatorv1.StaticPodOperatorStatus)
func UpdateStaticPodStatus(ctx context.Context, client StaticPodOperatorClient, updateFuncs ...UpdateStaticPodStatusFunc) (*operatorv1.StaticPodOperatorStatus, bool, error) {
updated := false
var updatedOperatorStatus *operatorv1.StaticPodOperatorStatus
numberOfAttempts := 0
err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
_, oldStatus, resourceVersion, err := client.GetStaticPodOperatorState()
defer func() {
numberOfAttempts++
}()
var oldStatus *operatorv1.StaticPodOperatorStatus
var resourceVersion string
var err error

if numberOfAttempts < 1 { // prefer lister if we haven't already failed.
_, oldStatus, resourceVersion, err = client.GetStaticPodOperatorState()

} else { // if we have failed enough times (chose 1 as a starting point, do a live GET
_, oldStatus, resourceVersion, err = client.GetStaticPodOperatorStateWithQuorum(ctx)
}
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/operator/v1helpers/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ type OperatorClient interface {
GetObjectMeta() (meta *metav1.ObjectMeta, err error)
// GetOperatorState returns the operator spec, status and the resource version, potentially from a lister.
GetOperatorState() (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error)
// GetOperatorStateWithQuorum return the operator spec, status and resource version directly from a server read.
GetOperatorStateWithQuorum(ctx context.Context) (spec *operatorv1.OperatorSpec, status *operatorv1.OperatorStatus, resourceVersion string, err error)
// UpdateOperatorSpec updates the spec of the operator, assuming the given resource version.
UpdateOperatorSpec(ctx context.Context, oldResourceVersion string, in *operatorv1.OperatorSpec) (out *operatorv1.OperatorSpec, newResourceVersion string, err error)
// UpdateOperatorStatus updates the status of the operator, assuming the given resource version.
Expand Down
11 changes: 11 additions & 0 deletions pkg/operator/v1helpers/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ func (c *fakeStaticPodOperatorClient) GetStaticPodOperatorState() (*operatorv1.S
return c.fakeStaticPodOperatorSpec, c.fakeStaticPodOperatorStatus, c.resourceVersion, nil
}

func (c *fakeStaticPodOperatorClient) GetLiveStaticPodOperatorState() (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
return c.GetStaticPodOperatorState()
}

func (c *fakeStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx context.Context) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) {
return c.fakeStaticPodOperatorSpec, c.fakeStaticPodOperatorStatus, c.resourceVersion, nil
}
Expand Down Expand Up @@ -154,6 +158,9 @@ func (c *fakeStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Co
func (c *fakeStaticPodOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
return &c.fakeStaticPodOperatorSpec.OperatorSpec, &c.fakeStaticPodOperatorStatus.OperatorStatus, c.resourceVersion, nil
}
func (c *fakeStaticPodOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
return c.GetOperatorState()
}
func (c *fakeStaticPodOperatorClient) UpdateOperatorSpec(ctx context.Context, s string, p *operatorv1.OperatorSpec) (spec *operatorv1.OperatorSpec, resourceVersion string, err error) {
panic("not supported")
}
Expand Down Expand Up @@ -241,6 +248,10 @@ func (c *fakeOperatorClient) GetOperatorState() (*operatorv1.OperatorSpec, *oper
return c.fakeOperatorSpec, c.fakeOperatorStatus, c.resourceVersion, nil
}

func (c *fakeOperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operatorv1.OperatorSpec, *operatorv1.OperatorStatus, string, error) {
return c.GetOperatorState()
}

func (c *fakeOperatorClient) UpdateOperatorStatus(ctx context.Context, resourceVersion string, status *operatorv1.OperatorStatus) (*operatorv1.OperatorStatus, error) {
if c.resourceVersion != resourceVersion {
return nil, errors.NewConflict(schema.GroupResource{Group: operatorv1.GroupName, Resource: "TestOperatorConfig"}, "instance", fmt.Errorf("invalid resourceVersion"))
Expand Down