Add default container annotation (#4031)

* Add default container annotation Since K8s 1.21, the kubectl.kubernetes.io/default-container annotation can be added to set a default container, removing the need for a -c flag or eliminating the message about defaulting Note: the `-c` flag can still be used to refer to a different, non-default container. Issues: [PGO-1941]
CrunchyData · Nov 20, 2024 · 1a009c2 · 1a009c2
1 parent ec99117
commit 1a009c2
Show file tree

Hide file tree

Showing 12 changed files with 76 additions and 10 deletions.
diff --git a/internal/controller/pgupgrade/jobs.go b/internal/controller/pgupgrade/jobs.go
@@ -16,6 +16,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 
 	"github.com/crunchydata/postgres-operator/internal/initialize"
+	"github.com/crunchydata/postgres-operator/internal/naming"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
@@ -129,7 +130,6 @@ func (r *PGUpgradeReconciler) generateUpgradeJob(
 	job.Namespace = upgrade.Namespace
 	job.Name = pgUpgradeJob(upgrade).Name
 
-	job.Annotations = upgrade.Spec.Metadata.GetAnnotationsOrNil()
 	job.Labels = Merge(upgrade.Spec.Metadata.GetLabelsOrNil(),
 		commonLabels(pgUpgrade, upgrade), //FIXME role pgupgrade
 		map[string]string{
@@ -145,6 +145,11 @@ func (r *PGUpgradeReconciler) generateUpgradeJob(
 		}
 	}
 
+	job.Annotations = Merge(upgrade.Spec.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: database.Name,
+		})
+
 	// Copy the pod template from the startup instance StatefulSet. This includes
 	// the service account, volumes, DNS policies, and scheduling constraints.
 	startup.Spec.Template.DeepCopyInto(&job.Spec.Template)
@@ -241,7 +246,6 @@ func (r *PGUpgradeReconciler) generateRemoveDataJob(
 	job.Namespace = upgrade.Namespace
 	job.Name = upgrade.Name + "-" + sts.Name
 
-	job.Annotations = upgrade.Spec.Metadata.GetAnnotationsOrNil()
 	job.Labels = labels.Merge(upgrade.Spec.Metadata.GetLabelsOrNil(),
 		commonLabels(removeData, upgrade)) //FIXME role removedata
 
@@ -254,6 +258,11 @@ func (r *PGUpgradeReconciler) generateRemoveDataJob(
 		}
 	}
 
+	job.Annotations = Merge(upgrade.Spec.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: database.Name,
+		})
+
 	// Copy the pod template from the sts instance StatefulSet. This includes
 	// the service account, volumes, DNS policies, and scheduling constraints.
 	sts.Spec.Template.DeepCopyInto(&job.Spec.Template)

diff --git a/internal/controller/pgupgrade/jobs_test.go b/internal/controller/pgupgrade/jobs_test.go
@@ -62,6 +62,8 @@ func TestGenerateUpgradeJob(t *testing.T) {
 apiVersion: batch/v1
 kind: Job
 metadata:
+  annotations:
+    kubectl.kubernetes.io/default-container: database
   creationTimestamp: null
   labels:
     postgres-operator.crunchydata.com/cluster: pg5
@@ -81,6 +83,8 @@ spec:
   backoffLimit: 0
   template:
     metadata:
+      annotations:
+        kubectl.kubernetes.io/default-container: database
       creationTimestamp: null
       labels:
         postgres-operator.crunchydata.com/cluster: pg5
@@ -193,6 +197,8 @@ func TestGenerateRemoveDataJob(t *testing.T) {
 apiVersion: batch/v1
 kind: Job
 metadata:
+  annotations:
+    kubectl.kubernetes.io/default-container: database
   creationTimestamp: null
   labels:
     postgres-operator.crunchydata.com/cluster: pg5
@@ -211,6 +217,8 @@ spec:
   backoffLimit: 0
   template:
     metadata:
+      annotations:
+        kubectl.kubernetes.io/default-container: database
       creationTimestamp: null
       labels:
         postgres-operator.crunchydata.com/cluster: pg5

diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go
@@ -1265,6 +1265,9 @@ func generateInstanceStatefulSetIntent(_ context.Context,
 	sts.Spec.Template.Annotations = naming.Merge(
 		cluster.Spec.Metadata.GetAnnotationsOrNil(),
 		spec.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: naming.ContainerDatabase,
+		},
 	)
 	sts.Spec.Template.Labels = naming.Merge(
 		cluster.Spec.Metadata.GetLabelsOrNil(),

diff --git a/internal/controller/postgrescluster/pgadmin.go b/internal/controller/postgrescluster/pgadmin.go
@@ -258,7 +258,11 @@ func (r *Reconciler) reconcilePGAdminStatefulSet(
 	}
 	sts.Spec.Template.Annotations = naming.Merge(
 		cluster.Spec.Metadata.GetAnnotationsOrNil(),
-		cluster.Spec.UserInterface.PGAdmin.Metadata.GetAnnotationsOrNil())
+		cluster.Spec.UserInterface.PGAdmin.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: naming.ContainerPGAdmin,
+		},
+	)
 	sts.Spec.Template.Labels = naming.Merge(
 		cluster.Spec.Metadata.GetLabelsOrNil(),
 		cluster.Spec.UserInterface.PGAdmin.Metadata.GetLabelsOrNil(),

diff --git a/internal/controller/postgrescluster/pgadmin_test.go b/internal/controller/postgrescluster/pgadmin_test.go
@@ -500,6 +500,8 @@ func TestReconcilePGAdminStatefulSet(t *testing.T) {
 		template.Spec.Volumes = nil
 
 		assert.Assert(t, cmp.MarshalMatches(template.ObjectMeta, `
+annotations:
+  kubectl.kubernetes.io/default-container: pgadmin
 creationTimestamp: null
 labels:
   postgres-operator.crunchydata.com/cluster: test-cluster
@@ -613,6 +615,7 @@ terminationGracePeriodSeconds: 30
 		assert.Assert(t, cmp.MarshalMatches(template.ObjectMeta, `
 annotations:
   annotation1: annotationvalue
+  kubectl.kubernetes.io/default-container: pgadmin
 creationTimestamp: null
 labels:
   label1: labelvalue

diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go
@@ -572,7 +572,11 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster
 
 	annotations := naming.Merge(
 		postgresCluster.Spec.Metadata.GetAnnotationsOrNil(),
-		postgresCluster.Spec.Backups.PGBackRest.Metadata.GetAnnotationsOrNil())
+		postgresCluster.Spec.Backups.PGBackRest.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: naming.PGBackRestRepoContainerName,
+		},
+	)
 	labels := naming.Merge(
 		postgresCluster.Spec.Metadata.GetLabelsOrNil(),
 		postgresCluster.Spec.Backups.PGBackRest.Metadata.GetLabelsOrNil(),
@@ -792,6 +796,14 @@ func generateBackupJobSpecIntent(ctx context.Context, postgresCluster *v1beta1.P
 		container.Resources = postgresCluster.Spec.Backups.PGBackRest.Jobs.Resources
 	}
 
+	if annotations != nil {
+		annotations[naming.DefaultContainerAnnotation] = naming.PGBackRestRepoContainerName
+	} else {
+		annotations = map[string]string{
+			naming.DefaultContainerAnnotation: naming.PGBackRestRepoContainerName,
+		}
+	}
+
 	jobSpec := &batchv1.JobSpec{
 		Template: corev1.PodTemplateSpec{
 			ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: annotations},
@@ -1259,7 +1271,10 @@ func (r *Reconciler) generateRestoreJobIntent(cluster *v1beta1.PostgresCluster,
 	annotations := naming.Merge(
 		cluster.Spec.Metadata.GetAnnotationsOrNil(),
 		cluster.Spec.Backups.PGBackRest.Metadata.GetAnnotationsOrNil(),
-		map[string]string{naming.PGBackRestConfigHash: configHash})
+		map[string]string{
+			naming.PGBackRestConfigHash:       configHash,
+			naming.DefaultContainerAnnotation: naming.PGBackRestRestoreContainerName,
+		})
 	labels := naming.Merge(
 		cluster.Spec.Metadata.GetLabelsOrNil(),
 		cluster.Spec.Backups.PGBackRest.Metadata.GetLabelsOrNil(),

diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go
@@ -371,7 +371,11 @@ func (r *Reconciler) generatePGBouncerDeployment(
 	}
 	deploy.Spec.Template.Annotations = naming.Merge(
 		cluster.Spec.Metadata.GetAnnotationsOrNil(),
-		cluster.Spec.Proxy.PGBouncer.Metadata.GetAnnotationsOrNil())
+		cluster.Spec.Proxy.PGBouncer.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: naming.ContainerPGBouncer,
+		},
+	)
 	deploy.Spec.Template.Labels = naming.Merge(
 		cluster.Spec.Metadata.GetLabelsOrNil(),
 		cluster.Spec.Proxy.PGBouncer.Metadata.GetLabelsOrNil(),

diff --git a/internal/controller/postgrescluster/pgbouncer_test.go b/internal/controller/postgrescluster/pgbouncer_test.go
@@ -443,6 +443,7 @@ namespace: ns3
 		// Annotations present in the pod template.
 		assert.DeepEqual(t, deploy.Spec.Template.Annotations, map[string]string{
 			"a": "v1",
+			"kubectl.kubernetes.io/default-container": "pgbouncer",
 		})
 
 		// Labels present in the pod template.

diff --git a/internal/controller/postgrescluster/volumes.go b/internal/controller/postgrescluster/volumes.go
@@ -469,7 +469,9 @@ func (r *Reconciler) reconcileMovePGDataDir(ctx context.Context,
 
 	jobSpec := &batchv1.JobSpec{
 		Template: corev1.PodTemplateSpec{
-			ObjectMeta: metav1.ObjectMeta{Labels: labels},
+			ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: map[string]string{
+				naming.DefaultContainerAnnotation: naming.ContainerJobMovePGDataDir,
+			}},
 			Spec: corev1.PodSpec{
 				// Set the image pull secrets, if any exist.
 				// This is set here rather than using the service account due to the lack
@@ -586,7 +588,9 @@ func (r *Reconciler) reconcileMoveWALDir(ctx context.Context,
 
 	jobSpec := &batchv1.JobSpec{
 		Template: corev1.PodTemplateSpec{
-			ObjectMeta: metav1.ObjectMeta{Labels: labels},
+			ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: map[string]string{
+				naming.DefaultContainerAnnotation: naming.ContainerJobMovePGWALDir,
+			}},
 			Spec: corev1.PodSpec{
 				// Set the image pull secrets, if any exist.
 				// This is set here rather than using the service account due to the lack
@@ -708,7 +712,9 @@ func (r *Reconciler) reconcileMoveRepoDir(ctx context.Context,
 
 	jobSpec := &batchv1.JobSpec{
 		Template: corev1.PodTemplateSpec{
-			ObjectMeta: metav1.ObjectMeta{Labels: labels},
+			ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: map[string]string{
+				naming.DefaultContainerAnnotation: naming.ContainerJobMovePGBackRestRepoDir,
+			}},
 			Spec: corev1.PodSpec{
 				// Set the image pull secrets, if any exist.
 				// This is set here rather than using the service account due to the lack

diff --git a/internal/controller/standalone_pgadmin/statefulset.go b/internal/controller/standalone_pgadmin/statefulset.go
@@ -74,7 +74,12 @@ func statefulset(
 	sts.Spec.Selector = &metav1.LabelSelector{
 		MatchLabels: naming.StandalonePGAdminLabels(pgadmin.Name),
 	}
-	sts.Spec.Template.Annotations = pgadmin.Spec.Metadata.GetAnnotationsOrNil()
+	sts.Spec.Template.Annotations = naming.Merge(
+		pgadmin.Spec.Metadata.GetAnnotationsOrNil(),
+		map[string]string{
+			naming.DefaultContainerAnnotation: naming.ContainerPGAdmin,
+		},
+	)
 	sts.Spec.Template.Labels = naming.Merge(
 		pgadmin.Spec.Metadata.GetLabelsOrNil(),
 		naming.StandalonePGAdminDataLabels(pgadmin.Name),

diff --git a/internal/controller/standalone_pgadmin/statefulset_test.go b/internal/controller/standalone_pgadmin/statefulset_test.go
@@ -73,6 +73,8 @@ func TestReconcilePGAdminStatefulSet(t *testing.T) {
 		template.Spec.Volumes = nil
 
 		assert.Assert(t, cmp.MarshalMatches(template.ObjectMeta, `
+annotations:
+  kubectl.kubernetes.io/default-container: pgadmin
 creationTimestamp: null
 labels:
   postgres-operator.crunchydata.com/data: pgadmin
@@ -170,6 +172,7 @@ terminationGracePeriodSeconds: 30
 		assert.Assert(t, cmp.MarshalMatches(template.ObjectMeta, `
 annotations:
   annotation1: annotationvalue
+  kubectl.kubernetes.io/default-container: pgadmin
 creationTimestamp: null
 labels:
   label1: labelvalue

diff --git a/internal/naming/annotations.go b/internal/naming/annotations.go
@@ -68,4 +68,9 @@ const (
 	// to a cluster without backups. As usual with the operator, we do not
 	// touch cloud-based backups.
 	AuthorizeBackupRemovalAnnotation = annotationPrefix + "authorizeBackupRemoval"
+
+	// Used from Kubernetes v1.21+ to define a default container used when the
+	// `-c` flag is not passed.
+	// --https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container
+	DefaultContainerAnnotation = "kubectl.kubernetes.io/default-container"
 )