Add PriorityClass setting to registry pods for default CatalogSource

The registry pods may need to have necessary priorityclass settings.
OLM will set the priorityclass setting for registry pods by using
the priorityclass annotations in the default catalogsources.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

Diff for: pkg/controller/registry/reconciler/configmap_test.go

@@ -204,8 +204,9 @@ func objectsForCatalogSource(catsrc *v1alpha1.CatalogSource) []runtime.Object {
 		if catsrc.Spec.Image != "" {
 			decorated := grpcCatalogSourceDecorator{catsrc}
 			objs = clientfake.AddSimpleGeneratedNames(
-				decorated.Pod(""),
+				decorated.Pod(catsrc.GetName()),
 				decorated.Service(),
+				decorated.ServiceAccount(),
 			)
 		}
 	}

Diff for: pkg/controller/registry/reconciler/grpc.go

@@ -171,15 +171,16 @@ func (c *GrpcRegistryReconciler) currentUpdatePods(source grpcCatalogSourceDecor
 	return pods
 }
 
-func (c *GrpcRegistryReconciler) currentPodsWithCorrectImage(source grpcCatalogSourceDecorator) []*corev1.Pod {
+func (c *GrpcRegistryReconciler) currentPodsWithCorrectImageAndSpec(source grpcCatalogSourceDecorator, saName string) []*corev1.Pod {
 	pods, err := c.Lister.CoreV1().PodLister().Pods(source.GetNamespace()).List(labels.SelectorFromValidatedSet(source.Labels()))
 	if err != nil {
 		logrus.WithError(err).Warn("couldn't find pod in cache")
 		return nil
 	}
 	found := []*corev1.Pod{}
+	newPod := source.Pod(saName)
 	for _, p := range pods {
-		if p.Spec.Containers[0].Image == source.Spec.Image {
+		if p.Spec.Containers[0].Image == source.Spec.Image && podHashMatch(p, newPod) {
 			found = append(found, p)
 		}
 	}
@@ -192,11 +193,12 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.Ca
 
 	// if service status is nil, we force create every object to ensure they're created the first time
 	overwrite := source.Status.RegistryServiceStatus == nil
-	// recreate the pod if no existing pod is serving the latest image
-	overwritePod := overwrite || len(c.currentPodsWithCorrectImage(source)) == 0
 
 	//TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated)
 	sa, err := c.ensureSA(source)
+	// recreate the pod if no existing pod is serving the latest image or correct spec
+	overwritePod := overwrite || len(c.currentPodsWithCorrectImageAndSpec(source, sa.GetName())) == 0
+
 	if err != nil && !k8serror.IsAlreadyExists(err) {
 		return errors.Wrapf(err, "error ensuring service account: %s", source.GetName())
 	}
@@ -421,10 +423,9 @@ func (c *GrpcRegistryReconciler) removePods(pods []*corev1.Pod, namespace string
 // CheckRegistryServer returns true if the given CatalogSource is considered healthy; false otherwise.
 func (c *GrpcRegistryReconciler) CheckRegistryServer(catalogSource *v1alpha1.CatalogSource) (healthy bool, err error) {
 	source := grpcCatalogSourceDecorator{catalogSource}
-
 	// Check on registry resources
 	// TODO: add gRPC health check
-	if len(c.currentPodsWithCorrectImage(source)) < 1 ||
+	if len(c.currentPodsWithCorrectImageAndSpec(source, source.ServiceAccount().GetName())) < 1 ||
 		c.currentService(source) == nil {
 		healthy = false
 		return
@@ -478,3 +479,30 @@ func (c *GrpcRegistryReconciler) podFailed(pod *corev1.Pod) (bool, error) {
 	}
 	return false, nil
 }
+
+// podHashMatch will check the hash info in existing pod to ensure its
+// hash info matches the desired Service's hash.
+func podHashMatch(existing, new *corev1.Pod) bool {
+	labels := existing.GetLabels()
+	newLabels := new.GetLabels()
+	// If both new & existing pods don't have labels, consider it not matched
+	if len(labels) == 0 || len(newLabels) == 0 {
+		return false
+	}
+
+	existingPodSpecHash, ok := labels[PodHashLabelKey]
+	if !ok {
+		return false
+	}
+
+	newPodSpecHash, ok := newLabels[PodHashLabelKey]
+	if !ok {
+		return false
+	}
+
+	if existingPodSpecHash != newPodSpecHash {
+		return false
+	}
+
+	return true
+}

Diff for: pkg/controller/registry/reconciler/grpc_test.go

@@ -339,6 +339,74 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 	}
 }
 
+func TestRegistryPodPriorityClass(t *testing.T) {
+	now := func() metav1.Time { return metav1.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC) }
+
+	type cluster struct {
+		k8sObjs []runtime.Object
+	}
+	type in struct {
+		cluster cluster
+		catsrc  *v1alpha1.CatalogSource
+	}
+	tests := []struct {
+		testName      string
+		in            in
+		priorityclass string
+	}{
+		{
+			testName: "Grpc/WithValidPriorityClassAnnotation",
+			in: in{
+				catsrc: grpcCatalogSourceWithAnnotations(map[string]string{
+					"operatorframework.io/priorityclass": "system-cluster-critical",
+				}),
+			},
+			priorityclass: "system-cluster-critical",
+		},
+		{
+			testName: "Grpc/WithInvalidPriorityClassAnnotation",
+			in: in{
+				catsrc: grpcCatalogSourceWithAnnotations(map[string]string{
+					"operatorframework.io/priorityclass": "",
+				}),
+			},
+			priorityclass: "",
+		},
+		{
+			testName: "Grpc/WithNoPriorityClassAnnotation",
+			in: in{
+				catsrc: grpcCatalogSourceWithAnnotations(map[string]string{
+					"annotationkey": "annotationvalue",
+				}),
+			},
+			priorityclass: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.testName, func(t *testing.T) {
+			stopc := make(chan struct{})
+			defer close(stopc)
+
+			factory, client := fakeReconcilerFactory(t, stopc, withNow(now), withK8sObjs(tt.in.cluster.k8sObjs...), withK8sClientOptions(clientfake.WithNameGeneration(t)))
+			rec := factory.ReconcilerForSource(tt.in.catsrc)
+
+			err := rec.EnsureRegistryServer(tt.in.catsrc)
+			require.NoError(t, err)
+
+			// Check for resource existence
+			decorated := grpcCatalogSourceDecorator{tt.in.catsrc}
+			pod := decorated.Pod(tt.in.catsrc.GetName())
+			listOptions := metav1.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{CatalogSourceLabelKey: tt.in.catsrc.GetName()}).String()}
+			outPods, podErr := client.KubernetesInterface().CoreV1().Pods(pod.GetNamespace()).List(context.TODO(), listOptions)
+			require.NoError(t, podErr)
+			require.Len(t, outPods.Items, 1)
+			outPod := outPods.Items[0]
+			require.Equal(t, tt.priorityclass, outPod.Spec.PriorityClassName)
+			require.Equal(t, pod.GetLabels()[PodHashLabelKey], outPod.GetLabels()[PodHashLabelKey])
+		})
+	}
+}
+
 func TestGrpcRegistryChecker(t *testing.T) {
 	type cluster struct {
 		k8sObjs []runtime.Object

Diff for: pkg/controller/registry/reconciler/reconciler.go

@@ -2,14 +2,18 @@
 package reconciler
 
 import (
+	"fmt"
+	"hash/fnv"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/rand"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	controllerclient "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/controller-runtime/client"
+	hashutil "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/util/hash"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
 )
@@ -19,6 +23,10 @@ type nowFunc func() metav1.Time
 const (
 	// CatalogSourceLabelKey is the key for a label containing a CatalogSource name.
 	CatalogSourceLabelKey string = "olm.catalogSource"
+	// CatalogPriorityClassKey is the key of an annotation in default catalogsources
+	CatalogPriorityClassKey string = "operatorframework.io/priorityclass"
+	// PodHashLabelKey is the key of a label for podspec hash information
+	PodHashLabelKey = "olm.pod-spec-hash"
 )
 
 // RegistryEnsurer describes methods for ensuring a registry exists.
@@ -160,5 +168,25 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, saName strin
 			ServiceAccountName: saName,
 		},
 	}
+
+	// Set priorityclass if its annotation exists
+	if prio, ok := annotations[CatalogPriorityClassKey]; ok && prio != "" {
+		pod.Spec.PriorityClassName = prio
+	}
+
+	// Add PodSpec hash
+	// This hash info will be used to detect PodSpec changes
+	if labels == nil {
+		labels = make(map[string]string)
+	}
+	labels[PodHashLabelKey] = hashPodSpec(pod.Spec)
+	pod.SetLabels(labels)
 	return pod
 }
+
+// hashPodSpec calculates a hash given a copy of the pod spec
+func hashPodSpec(spec v1.PodSpec) string {
+	hasher := fnv.New32a()
+	hashutil.DeepHashObject(hasher, &spec)
+	return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
+}