Merge pull request #2042 from lunarwhite/np-spec-reconcile

CM-758: (resourceapply) fix NetworkPolicy spec not reconciling on update and potential infinite loops

Author: openshift-merge-bot[bot]

pkg/operator/resource/resourceapply/generic.go

@@ -147,7 +147,7 @@ func ApplyDirectly(ctx context.Context, clients *ClientHolder, recorder events.R
 			if clients.kubeClient == nil {
 				result.Error = fmt.Errorf("missing kubeClient")
 			} else {
-				result.Result, result.Changed, result.Error = ApplyNetworkPolicy(ctx, clients.kubeClient.NetworkingV1(), recorder, t)
+				result.Result, result.Changed, result.Error = ApplyNetworkPolicy(ctx, clients.kubeClient.NetworkingV1(), recorder, t, cache)
 			}
 		case *rbacv1.ClusterRole:
 			if clients.kubeClient == nil {

pkg/operator/resource/resourceapply/networking.go

@@ -15,34 +15,44 @@ import (
 	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
 )
 
-// ApplyClusterRole merges objectmeta, does not worry about anything else
-func ApplyNetworkPolicy(ctx context.Context, client networkingclientv1.NetworkPoliciesGetter, recorder events.Recorder, required *networkingv1.NetworkPolicy) (*networkingv1.NetworkPolicy, bool, error) {
+// ApplyNetworkPolicy merges objectmeta and requires spec
+func ApplyNetworkPolicy(ctx context.Context, client networkingclientv1.NetworkPoliciesGetter, recorder events.Recorder, required *networkingv1.NetworkPolicy, cache ResourceCache) (*networkingv1.NetworkPolicy, bool, error) {
 	existing, err := client.NetworkPolicies(required.Namespace).Get(ctx, required.Name, metav1.GetOptions{})
 	if apierrors.IsNotFound(err) {
 		requiredCopy := required.DeepCopy()
 		actual, err := client.NetworkPolicies(required.Namespace).Create(
 			ctx, resourcemerge.WithCleanLabelsAndAnnotations(requiredCopy).(*networkingv1.NetworkPolicy), metav1.CreateOptions{})
 		resourcehelper.ReportCreateEvent(recorder, required, err)
+		cache.UpdateCachedResourceMetadata(required, actual)
 		return actual, true, err
 	}
 	if err != nil {
 		return nil, false, err
 	}
 
+	if cache.SafeToSkipApply(required, existing) {
+		return existing, false, nil
+	}
+
 	modified := false
 	existingCopy := existing.DeepCopy()
 
 	resourcemerge.EnsureObjectMeta(&modified, &existingCopy.ObjectMeta, required.ObjectMeta)
-	if equality.Semantic.DeepEqual(existingCopy.Spec, required.Spec) && !modified {
+	specContentSame := equality.Semantic.DeepEqual(existingCopy.Spec, required.Spec)
+	if specContentSame && !modified {
+		cache.UpdateCachedResourceMetadata(required, existingCopy)
 		return existingCopy, false, nil
 	}
 
+	existingCopy.Spec = required.Spec
+
 	if klog.V(2).Enabled() {
 		klog.Infof("NetworkPolicy %q changes: %v", required.Name, JSONPatchNoError(existing, existingCopy))
 	}
 
 	actual, err := client.NetworkPolicies(existingCopy.Namespace).Update(ctx, existingCopy, metav1.UpdateOptions{})
 	resourcehelper.ReportUpdateEvent(recorder, required, err)
+	cache.UpdateCachedResourceMetadata(required, actual)
 	return actual, true, err
 }