provisioner also removes annotations

Signed-off-by: Sunjay Bhatia <[email protected]>
projectcontour · May 9, 2024 · c1c4553 · c1c4553
1 parent b79084b
commit c1c4553
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 56 deletions.
diff --git a/changelogs/unreleased/6269-sunjayBhatia-deprecation.md b/changelogs/unreleased/6269-sunjayBhatia-deprecation.md
@@ -1,4 +1,4 @@
 ## Contour sample YAML manifests no longer use `prometheus.io/` annotations
 
-The annotations for notifying a Prometheus instance on how to scrape metrics from Contour and Envoy pods have been removed.
+The annotations for notifying a Prometheus instance on how to scrape metrics from Contour and Envoy pods have been removed from the deployment YAMLs and the Gateway provisioner.
 The suggested mechanism for doing so now is to use [kube-prometheus](https://github.com/prometheus-operator/kube-prometheus) and the [`PodMonitor`](https://prometheus-operator.dev/docs/operator/design/#podmonitor) resource.
diff --git a/internal/provisioner/controller/gateway_test.go b/internal/provisioner/controller/gateway_test.go
@@ -1098,7 +1098,7 @@ func TestGatewayReconcile(t *testing.T) {
 					},
 				}
 				require.NoError(t, r.client.Get(context.Background(), keyFor(ds), ds))
-				assert.Contains(t, ds.Spec.Template.ObjectMeta.Annotations, "key", "prometheus.io/scrape", "prometheus.io/port")
+				assert.Contains(t, ds.Spec.Template.ObjectMeta.Annotations, "key")
 			},
 		},
 
@@ -1212,7 +1212,7 @@ func TestGatewayReconcile(t *testing.T) {
 				}
 
 				require.NoError(t, r.client.Get(context.Background(), keyFor(deploy), deploy))
-				assert.Contains(t, deploy.Spec.Template.ObjectMeta.Annotations, "key", "prometheus.io/scrape", "prometheus.io/port")
+				assert.Contains(t, deploy.Spec.Template.ObjectMeta.Annotations, "key")
 			},
 		},
 

diff --git a/internal/provisioner/model/model.go b/internal/provisioner/model/model.go
@@ -223,11 +223,9 @@ type ContourSpec struct {
 	EnvoyExtraVolumeMounts []core_v1.VolumeMount
 
 	// EnvoyPodAnnotations holds the annotations that will be add to the envoy‘s pod.
-	// the annotations: "prometheus.io/scrape", "prometheus.io/port", "prometheus.io/path" will be overwritten with predefined value.
 	EnvoyPodAnnotations map[string]string
 
 	// ContourPodAnnotations holds the annotations that will be add to the contour's pod.
-	// the annotations: "prometheus.io/scrape", "prometheus.io/port" will be overwritten with predefined value.
 	ContourPodAnnotations map[string]string
 
 	// Compute Resources required by envoy container.

diff --git a/internal/provisioner/objects/dataplane/dataplane.go b/internal/provisioner/objects/dataplane/dataplane.go
@@ -351,8 +351,6 @@ func DesiredDaemonSet(contour *model.Contour, contourImage, envoyImage string) *
 			UpdateStrategy: contour.Spec.EnvoyDaemonSetUpdateStrategy,
 			Template: core_v1.PodTemplateSpec{
 				ObjectMeta: meta_v1.ObjectMeta{
-					// TODO [danehans]: Remove the prometheus annotations when Contour is updated to
-					// show how the Prometheus Operator is used to scrape Contour/Envoy metrics.
 					Annotations: envoyPodAnnotations(contour),
 					Labels:      envoyPodLabels(contour),
 				},
@@ -425,8 +423,6 @@ func desiredDeployment(contour *model.Contour, contourImage, envoyImage string)
 			Strategy: contour.Spec.EnvoyDeploymentStrategy,
 			Template: core_v1.PodTemplateSpec{
 				ObjectMeta: meta_v1.ObjectMeta{
-					// TODO [danehans]: Remove the prometheus annotations when Contour is updated to
-					// show how the Prometheus Operator is used to scrape Contour/Envoy metrics.
 					Annotations: envoyPodAnnotations(contour),
 					Labels:      envoyPodLabels(contour),
 				},
@@ -550,18 +546,6 @@ func envoyPodAnnotations(contour *model.Contour) map[string]string {
 		annotations[k] = v
 	}
 
-	metricsPort := objects.EnvoyMetricsPort
-	if contour.Spec.RuntimeSettings != nil &&
-		contour.Spec.RuntimeSettings.Envoy != nil &&
-		contour.Spec.RuntimeSettings.Envoy.Metrics != nil &&
-		contour.Spec.RuntimeSettings.Envoy.Metrics.Port > 0 {
-		metricsPort = int32(contour.Spec.RuntimeSettings.Envoy.Metrics.Port)
-	}
-
-	annotations["prometheus.io/scrape"] = "true"
-	annotations["prometheus.io/port"] = fmt.Sprint(metricsPort)
-	annotations["prometheus.io/path"] = "/stats/prometheus"
-
 	// Annotations specified on the Gateway take precedence
 	// over annotations specified on the GatewayClass/its parameters.
 	for k, v := range contour.CommonAnnotations() {

diff --git a/internal/provisioner/objects/dataplane/dataplane_test.go b/internal/provisioner/objects/dataplane/dataplane_test.go
@@ -25,6 +25,7 @@ import (
 	contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	"github.com/projectcontour/contour/internal/provisioner/model"
 	"github.com/projectcontour/contour/internal/provisioner/objects"
+	"github.com/stretchr/testify/require"
 )
 
 func checkDaemonSetHasEnvVar(t *testing.T, ds *apps_v1.DaemonSet, container, name string) {
@@ -55,6 +56,13 @@ func checkDaemonSetHasEnvVar(t *testing.T, ds *apps_v1.DaemonSet, container, nam
 	t.Errorf("daemonset is missing environment variable %q", name)
 }
 
+func checkDaemonSetHasAnnotation(t *testing.T, ds *apps_v1.DaemonSet, key, value string) {
+	t.Helper()
+
+	require.Contains(t, ds.Spec.Template.Annotations, key)
+	require.Equal(t, value, ds.Spec.Template.Annotations[key])
+}
+
 func checkDaemonSetHasContainer(t *testing.T, ds *apps_v1.DaemonSet, name string, expect bool) *core_v1.Container {
 	t.Helper()
 
@@ -251,15 +259,6 @@ func checkContainerHasReadinessPort(t *testing.T, container *core_v1.Container,
 	t.Errorf("container has unexpected readiness port %d", port)
 }
 
-func checkDaemonSetHasMetricsPort(t *testing.T, ds *apps_v1.DaemonSet, port int32) {
-	t.Helper()
-
-	if ds.Spec.Template.ObjectMeta.Annotations["prometheus.io/port"] == fmt.Sprint(port) {
-		return
-	}
-	t.Errorf("container has unexpected metrics port %d", port)
-}
-
 func checkEnvoyDeploymentHasAffinity(t *testing.T, d *apps_v1.Deployment, contour *model.Contour) {
 	t.Helper()
 	if apiequality.Semantic.DeepEqual(*d.Spec.Template.Spec.Affinity,
@@ -291,6 +290,9 @@ func TestDesiredDaemonSet(t *testing.T) {
 		"annotation":           "value",
 		"prometheus.io/scrape": "false",
 	}
+	cntr.Spec.ResourceAnnotations = map[string]string{
+		"other-annotation": "other-val",
+	}
 
 	volTest := core_v1.Volume{
 		Name: "vol-test-mount",
@@ -367,7 +369,9 @@ func TestDesiredDaemonSet(t *testing.T) {
 	checkDaemonSecurityContext(t, ds)
 	checkDaemonSetHasVolume(t, ds, volTest, volTestMount)
 	checkDaemonSetHasPodAnnotations(t, ds, envoyPodAnnotations(cntr))
-	checkDaemonSetHasMetricsPort(t, ds, objects.EnvoyMetricsPort)
+	checkDaemonSetHasAnnotation(t, ds, "annotation", "value")
+	checkDaemonSetHasAnnotation(t, ds, "other-annotation", "other-val")
+	checkDaemonSetHasAnnotation(t, ds, "prometheus.io/scrape", "false")
 
 	checkDaemonSetHasResourceRequirements(t, ds, resQutoa)
 	checkDaemonSetHasUpdateStrategy(t, ds, cntr.Spec.EnvoyDaemonSetUpdateStrategy)
@@ -430,7 +434,6 @@ func TestEnvoyCustomPorts(t *testing.T) {
 	testContourImage := "ghcr.io/projectcontour/contour:test"
 	testEnvoyImage := "docker.io/envoyproxy/envoy:test"
 	ds := DesiredDaemonSet(cntr, testContourImage, testEnvoyImage)
-	checkDaemonSetHasMetricsPort(t, ds, int32(metricPort))
 	checkContainerHasPort(t, ds, int32(metricPort))
 
 	container := checkDaemonSetHasContainer(t, ds, EnvoyContainerName, true)

diff --git a/internal/provisioner/objects/deployment/deployment.go b/internal/provisioner/objects/deployment/deployment.go
@@ -224,8 +224,6 @@ func DesiredDeployment(contour *model.Contour, image string) *apps_v1.Deployment
 			Strategy: contour.Spec.ContourDeploymentStrategy,
 			Template: core_v1.PodTemplateSpec{
 				ObjectMeta: meta_v1.ObjectMeta{
-					// TODO [danehans]: Remove the prometheus annotations when Contour is updated to
-					// show how the Prometheus Operator is used to scrape Contour/Envoy metrics.
 					Annotations: contourPodAnnotations(contour),
 					Labels:      contourPodLabels(contour),
 				},
@@ -322,16 +320,6 @@ func contourPodAnnotations(contour *model.Contour) map[string]string {
 		annotations[k] = v
 	}
 
-	port := metricsPort
-	if contour.Spec.RuntimeSettings != nil &&
-		contour.Spec.RuntimeSettings.Metrics != nil &&
-		contour.Spec.RuntimeSettings.Metrics.Port > 0 {
-		port = contour.Spec.RuntimeSettings.Metrics.Port
-	}
-
-	annotations["prometheus.io/scrape"] = "true"
-	annotations["prometheus.io/port"] = fmt.Sprint(port)
-
 	// Annotations specified on the Gateway take precedence
 	// over annotations specified on the GatewayClass/its parameters.
 	for k, v := range contour.CommonAnnotations() {

diff --git a/internal/provisioner/objects/deployment/deployment_test.go b/internal/provisioner/objects/deployment/deployment_test.go
@@ -26,6 +26,7 @@ import (
 	contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	"github.com/projectcontour/contour/internal/provisioner/model"
+	"github.com/stretchr/testify/require"
 )
 
 func checkDeploymentHasEnvVar(t *testing.T, deploy *apps_v1.Deployment, name string) {
@@ -70,14 +71,11 @@ func checkDeploymentHasLabels(t *testing.T, deploy *apps_v1.Deployment, expected
 	t.Errorf("deployment has unexpected %q labels", deploy.Labels)
 }
 
-func checkPodHasAnnotations(t *testing.T, tmpl *core_v1.PodTemplateSpec, annotations map[string]string) {
+func checkPodHasAnnotation(t *testing.T, tmpl core_v1.PodTemplateSpec, key, value string) {
 	t.Helper()
 
-	for k, v := range annotations {
-		if val, ok := tmpl.Annotations[k]; !ok || val != v {
-			t.Errorf("pod template has unexpected %q annotations", tmpl.Annotations)
-		}
-	}
+	require.Contains(t, tmpl.Annotations, key)
+	require.Equal(t, value, tmpl.Annotations[key])
 }
 
 func checkContainerHasArg(t *testing.T, container *core_v1.Container, arg string) {
@@ -153,10 +151,6 @@ func TestDesiredDeployment(t *testing.T) {
 		},
 	}
 
-	annotations := map[string]string{
-		"key":                  "value",
-		"prometheus.io/scrape": "false",
-	}
 	cntr.Spec.ContourResources = resQutoa
 
 	// Change the Kubernetes log level to test --kubernetes-debug.
@@ -168,7 +162,13 @@ func TestDesiredDeployment(t *testing.T) {
 	cntr.Spec.ResourceLabels = map[string]string{
 		"key": "value",
 	}
-	cntr.Spec.ContourPodAnnotations = annotations
+	cntr.Spec.ContourPodAnnotations = map[string]string{
+		"key":                  "value",
+		"prometheus.io/scrape": "false",
+	}
+	cntr.Spec.ResourceAnnotations = map[string]string{
+		"other-annotation": "other-val",
+	}
 
 	// Use non-default container ports to test that --envoy-service-http(s)-port
 	// flags are added.
@@ -185,7 +185,9 @@ func TestDesiredDeployment(t *testing.T) {
 	checkDeploymentHasEnvVar(t, deploy, contourNsEnvVar)
 	checkDeploymentHasEnvVar(t, deploy, contourPodEnvVar)
 	checkDeploymentHasLabels(t, deploy, cntr.WorkloadLabels())
-	checkPodHasAnnotations(t, &deploy.Spec.Template, contourPodAnnotations(cntr))
+	checkPodHasAnnotation(t, deploy.Spec.Template, "key", "value")
+	checkPodHasAnnotation(t, deploy.Spec.Template, "prometheus.io/scrape", "false")
+	checkPodHasAnnotation(t, deploy.Spec.Template, "other-annotation", "other-val")
 
 	for _, port := range cntr.Spec.NetworkPublishing.Envoy.Ports {
 		switch port.Name {