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

add test demonstrating zero-diff check failure #29101

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 198 additions & 0 deletions test/extended/operators/server_side_apply_client_copy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
package operators

import (
"context"
"fmt"
operatorv1 "github.com/openshift/api/operator/v1"
"k8s.io/apimachinery/pkg/api/equality"

applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
)

// TODO replace with the library-go impl once created
type dynamicOperatorClient struct {
gvk schema.GroupVersionKind
configName string
client dynamic.ResourceInterface

extractApplySpec StaticPodOperatorSpecExtractorFunc
extractApplyStatus StaticPodOperatorStatusExtractorFunc
}

type StaticPodOperatorSpecExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.StaticPodOperatorSpecApplyConfiguration, error)
type StaticPodOperatorStatusExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.StaticPodOperatorStatusApplyConfiguration, error)
type OperatorSpecExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorSpecApplyConfiguration, error)
type OperatorStatusExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorStatusApplyConfiguration, error)

func convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec OperatorSpecExtractorFunc) StaticPodOperatorSpecExtractorFunc {
return func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.StaticPodOperatorSpecApplyConfiguration, error) {
operatorSpec, err := extractApplySpec(obj, fieldManager)
if err != nil {
return nil, err
}
if operatorSpec == nil {
return nil, nil
}
return &applyoperatorv1.StaticPodOperatorSpecApplyConfiguration{
OperatorSpecApplyConfiguration: *operatorSpec,
}, nil
}
}

func convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus OperatorStatusExtractorFunc) StaticPodOperatorStatusExtractorFunc {
return func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.StaticPodOperatorStatusApplyConfiguration, error) {
operatorStatus, err := extractApplyStatus(obj, fieldManager)
if err != nil {
return nil, err
}
if operatorStatus == nil {
return nil, nil
}
return &applyoperatorv1.StaticPodOperatorStatusApplyConfiguration{
OperatorStatusApplyConfiguration: *operatorStatus,
}, nil
}
}

func (c dynamicOperatorClient) ApplyOperatorSpec(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorSpecApplyConfiguration) (err error) {
applyMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(applyConfiguration)
if err != nil {
return fmt.Errorf("failed to convert to unstructured: %w", err)
}

return c.applyOperatorSpec(ctx, fieldManager, applyMap)
}

func (c dynamicOperatorClient) applyOperatorSpec(ctx context.Context, fieldManager string, applyMap map[string]interface{}) (err error) {
applyUnstructured := &unstructured.Unstructured{
Object: map[string]interface{}{
"spec": applyMap,
},
}
applyUnstructured.SetGroupVersionKind(c.gvk)
applyUnstructured.SetName(c.configName)

original, err := c.client.Get(ctx, c.configName, metav1.GetOptions{})
switch {
case apierrors.IsNotFound(err):
// do nothing and proceed with the apply
case err != nil:
return fmt.Errorf("unable to read existing %q: %w", c.configName, err)
default:
if c.extractApplySpec == nil {
return fmt.Errorf("extractApplySpec is nil")
}
previouslySetFields, err := c.extractApplySpec(original, fieldManager)
if err != nil {
return fmt.Errorf("unable to extract spec for %q: %w", fieldManager, err)
}
currentApplyConfiguration := &applyoperatorv1.StaticPodOperatorSpecApplyConfiguration{}
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(applyMap, applyUnstructured); err != nil {
return fmt.Errorf("unable to convert to staticpodoperatorspec: %w", err)
}
if equality.Semantic.DeepEqual(previouslySetFields, currentApplyConfiguration) {
// nothing to apply, so return early
return nil
}
}

_, err = c.client.Apply(ctx, c.configName, applyUnstructured, metav1.ApplyOptions{
Force: true,
FieldManager: fieldManager,
})
if err != nil {
return fmt.Errorf("unable to Apply for operator using fieldManager %q: %w", fieldManager, err)
}

return nil
}

func (c dynamicOperatorClient) ApplyOperatorStatus(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorStatusApplyConfiguration) (string, error) {
if applyConfiguration == nil {
return "no input", fmt.Errorf("desired status must have value")
}
desiredConfiguration := applyoperatorv1.StaticPodOperatorStatus()
desiredConfiguration.OperatorStatusApplyConfiguration = *applyConfiguration
return c.applyOperatorStatus(ctx, fieldManager, desiredConfiguration)
}

func (c dynamicOperatorClient) applyOperatorStatus(ctx context.Context, fieldManager string, desiredConfiguration *applyoperatorv1.StaticPodOperatorStatusApplyConfiguration) (string, error) {
applyMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desiredConfiguration)
if err != nil {
return "", fmt.Errorf("failed to convert to unstructured: %w", err)
}
applyUnstructured := &unstructured.Unstructured{
Object: map[string]interface{}{
"status": applyMap,
},
}
applyUnstructured.SetGroupVersionKind(c.gvk)
applyUnstructured.SetName(c.configName)

original, err := c.client.Get(ctx, c.configName, metav1.GetOptions{})
switch {
case apierrors.IsNotFound(err):
// do nothing and proceed with the apply
case err != nil:
return "", fmt.Errorf("unable to read existing %q: %w", c.configName, err)
default:
if c.extractApplyStatus == nil {
return "", fmt.Errorf("extractApplyStatus is nil")
}
previouslySetFields, err := c.extractApplyStatus(original, fieldManager)
if err != nil {
return "", fmt.Errorf("unable to extract status for %q: %w", fieldManager, err)
}
if equality.Semantic.DeepEqual(previouslySetFields, desiredConfiguration) {
// nothing to apply, so return early
return "nothing to apply", nil
}
}

_, err = c.client.ApplyStatus(ctx, c.configName, applyUnstructured, metav1.ApplyOptions{
Force: true,
FieldManager: fieldManager,
})
if err != nil {
return "modification attempted", fmt.Errorf("unable to ApplyStatus for operator using fieldManager %q: %w", fieldManager, err)
}

return "modification done", nil
}

func extractOperatorSpec(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorSpecApplyConfiguration, error) {
castObj := &operatorv1.OpenShiftAPIServer{}
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, castObj); err != nil {
return nil, fmt.Errorf("unable to convert to OpenShiftAPIServer: %w", err)
}
ret, err := applyoperatorv1.ExtractOpenShiftAPIServer(castObj, fieldManager)
if err != nil {
return nil, fmt.Errorf("unable to extract fields for %q: %w", fieldManager, err)
}
if ret.Spec == nil {
return nil, nil
}
return &ret.Spec.OperatorSpecApplyConfiguration, nil
}

func extractOperatorStatus(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorStatusApplyConfiguration, error) {
castObj := &operatorv1.OpenShiftAPIServer{}
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, castObj); err != nil {
return nil, fmt.Errorf("unable to convert to OpenShiftAPIServer: %w", err)
}
ret, err := applyoperatorv1.ExtractOpenShiftAPIServerStatus(castObj, fieldManager)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I messed up this name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, so instead of extracting the status you were extracting the spec thus the diff!

if err != nil {
return nil, fmt.Errorf("unable to extract fields for %q: %w", fieldManager, err)
}

if ret.Status == nil {
return nil, nil
}
return &ret.Status.OperatorStatusApplyConfiguration, nil
}
69 changes: 69 additions & 0 deletions test/extended/operators/server_side_apply_zero_diff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package operators

import (
"context"
"fmt"
g "github.com/onsi/ginkgo/v2"
o "github.com/onsi/gomega"
operatorv1 "github.com/openshift/api/operator/v1"
applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
exutil "github.com/openshift/origin/test/extended/util"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
admissionapi "k8s.io/pod-security-admission/api"
"math/rand"
)

var _ = g.Describe("[sig-apimachinery]", func() {

defer g.GinkgoRecover()

oc := exutil.NewCLIWithPodSecurityLevel("server-side-apply-zero-diff", admissionapi.LevelPrivileged)

g.Describe("server-side-apply zero diff detection", func() {
g.It("should not update when the existing values have not changed", func() {
ctx := context.Background()
isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
o.Expect(err).NotTo(o.HaveOccurred())
if isMicroShift {
g.Skip("microshift lacks the API")
}

instanceName := fmt.Sprintf("test-instance-%d", rand.Int31())
gvr := operatorv1.GroupVersion.WithResource("openshiftapiservers")
client := oc.AdminDynamicClient().Resource(gvr)
dynamicOperatorClient := &dynamicOperatorClient{
gvk: operatorv1.GroupVersion.WithKind("OpenShiftAPIServer"),
configName: instanceName,
client: client,
extractApplySpec: convertOperatorSpecToStaticPodOperatorSpec(extractOperatorSpec),
extractApplyStatus: convertOperatorStatusToStaticPodOperatorStatus(extractOperatorStatus),
}

creatingApply := applyoperatorv1.OperatorSpec().WithLogLevel(operatorv1.Debug)
err = dynamicOperatorClient.ApplyOperatorSpec(ctx, "creator", creatingApply)
o.Expect(err).NotTo(o.HaveOccurred())
defer client.Delete(ctx, "test-instance", metav1.DeleteOptions{})

firstConditionInitial := applyoperatorv1.OperatorStatus().
WithConditions(applyoperatorv1.OperatorCondition().
WithType("First").
WithStatus(operatorv1.ConditionTrue).
WithReason("Error").
WithMessage("Whatever"))
action, err := dynamicOperatorClient.ApplyOperatorStatus(ctx, "first-condition-setter", firstConditionInitial)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(action).To(o.Equal("modification done"))

firstConditionIdenticalToInitial := applyoperatorv1.OperatorStatus().
WithConditions(applyoperatorv1.OperatorCondition().
WithType("First").
WithStatus(operatorv1.ConditionTrue).
WithReason("Error").
WithMessage("Whatever"))
action, err = dynamicOperatorClient.ApplyOperatorStatus(ctx, "first-condition-setter", firstConditionIdenticalToInitial)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(action).To(o.Equal("nothing to apply"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth adding one more case where we stop setting the condition and check if the first call produces a diff but the second doesn't ?

})

})
})

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.