regression: Set resource version for CRs

Set resource version for all `monitoring.go` GVRs. Earlier, this was not
done by the machinery responsible for it, which caused manifest
applications that had no resource version present to encounter the
following error:
```
metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
```
This patch addresses that regression, which was introduced originally in openshift#1575.

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>

pkg/operator/resource/resourceapply/monitoring.go

@@ -136,7 +136,7 @@ func ApplyUnstructuredResourceImproved(
 		return nil, false, err
 	}
 
-	// Check if the metadata objects differ.
+	// Check if the metadata objects differ. This only checks for selective fields (excluding the resource version, among others).
 	didMetadataModify := ptr.To(false)
 	resourcemerge.EnsureObjectMeta(didMetadataModify, &existingObjectMetaTyped, requiredObjectMetaTyped)
 
@@ -162,10 +162,14 @@ func ApplyUnstructuredResourceImproved(
 		klog.Infof("%s %q changes: %v", resourceGVR.String(), namespace+"/"+name, JSONPatchNoError(existing, existingCopy))
 	}
 
+	// CRs, unlike certain core objects (e.g., deployments), need to specify the resource version for updates.
+	requiredCopy := required.DeepCopy()
+	requiredCopy.SetResourceVersion(existingCopy.GetResourceVersion())
+
 	// Perform update if resource exists but different from the required (desired) one.
-	actual, err := client.Resource(resourceGVR).Namespace(namespace).Update(ctx, required, metav1.UpdateOptions{})
-	resourcehelper.ReportUpdateEvent(recorder, required, err)
-	cache.UpdateCachedResourceMetadata(required, actual)
+	actual, err := client.Resource(resourceGVR).Namespace(namespace).Update(ctx, requiredCopy, metav1.UpdateOptions{})
+	resourcehelper.ReportUpdateEvent(recorder, requiredCopy, err)
+	cache.UpdateCachedResourceMetadata(requiredCopy, actual)
 	return actual, true, err
 }
 

test/e2e-monitoring/OWNERS

@@ -0,0 +1,11 @@
+reviewers:
+  - dgrisonnet
+  - p0lyn0mial
+  - sttts
+  - tkashem
+  - rexagod
+approvers:
+  - dgrisonnet
+  - p0lyn0mial
+  - sttts
+  - tkashem

test/e2e-monitoring/monitoring_test.go

@@ -0,0 +1,125 @@
+package e2e_monitoring
+
+import (
+	"context"
+	"testing"
+
+	"github.com/openshift/library-go/pkg/operator/events"
+	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
+	"github.com/openshift/library-go/test/library"
+	monv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
+	"github.com/stretchr/testify/require"
+	"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/apimachinery/pkg/util/intstr"
+	"k8s.io/client-go/dynamic"
+)
+
+func TestResourceVersionApplication(t *testing.T) {
+	config, err := library.NewClientConfigForTest()
+	require.NoError(t, err)
+
+	// Define the resource.
+	duration := monv1.Duration("5m")
+	resource := monv1.PrometheusRule{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "monitoring.coreos.com/v1",
+			Kind:       "PrometheusRule",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-prometheusrule",
+			Namespace: "default",
+		},
+		Spec: monv1.PrometheusRuleSpec{
+			Groups: []monv1.RuleGroup{
+				{
+					Name: "example",
+					Rules: []monv1.Rule{
+						{
+							Alert: "Foo",
+							Expr:  intstr.FromString("foo > 0"),
+							For:   &duration,
+							Labels: map[string]string{
+								"bar": "baz",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	gvr := schema.GroupVersionResource{
+		Group:    resource.GroupVersionKind().Group,
+		Version:  resource.GroupVersionKind().Version,
+		Resource: "prometheusrules",
+	}
+
+	// Initialize pre-requisites.
+	cache := resourceapply.NewResourceCache()
+	recorder := events.NewInMemoryRecorder("TestResourceVersionApplication")
+	dynamicClient, err := dynamic.NewForConfig(config)
+	require.NoError(t, err)
+
+	// Create the resource.
+	unstructuredResource := &unstructured.Unstructured{}
+	unstructuredResourceMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&resource)
+	require.NoError(t, err)
+	unstructuredResource.SetUnstructuredContent(unstructuredResourceMap)
+	_, didUpdate, err := resourceapply.ApplyUnstructuredResourceImproved(
+		context.TODO(),
+		dynamicClient,
+		recorder,
+		unstructuredResource,
+		cache,
+		gvr,
+		nil,
+		nil,
+	)
+	if err != nil && !errors.IsAlreadyExists(err) {
+		t.Fatalf("Failed to create resource: %v", err)
+	}
+	require.True(t, didUpdate)
+
+	// Update the resource with a change, without specifying a resource version.
+	resource.Spec.Groups[0].Rules[0].Labels["bar"] = "qux"
+	unstructuredResourceMap, err = runtime.DefaultUnstructuredConverter.ToUnstructured(&resource)
+	require.NoError(t, err)
+	unstructuredResource.SetUnstructuredContent(unstructuredResourceMap)
+	_, didUpdate, err = resourceapply.ApplyUnstructuredResourceImproved(
+		context.TODO(),
+		dynamicClient,
+		recorder,
+		unstructuredResource,
+		cache,
+		gvr,
+		nil,
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("Failed to update resource: %v", err)
+	}
+	require.True(t, didUpdate)
+
+	// Update the resource without any changes, without specifying a resource version.
+	_, didUpdate, err = resourceapply.ApplyUnstructuredResourceImproved(
+		context.TODO(),
+		dynamicClient,
+		recorder,
+		unstructuredResource,
+		cache,
+		gvr,
+		nil,
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("Failed to update resource: %v", err)
+	}
+	require.False(t, didUpdate)
+
+	// Delete the resource.
+	err = dynamicClient.Resource(gvr).Namespace(resource.GetNamespace()).Delete(context.TODO(), resource.GetName(), metav1.DeleteOptions{})
+	require.NoError(t, err)
+}