From 9c1fce95680921fb6048d82bda038543222a7763 Mon Sep 17 00:00:00 2001 From: Ahmed Mezghani <38987709+ahmed-mez@users.noreply.github.com> Date: Mon, 31 Aug 2020 23:04:22 +0200 Subject: [PATCH] Support wpa configuration (#142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * support wpa config * make hasWpasRbacs more restrictive Co-authored-by: Lénaïc Huard --- .../templates/clusterrole.yaml | 8 ++ deploy/clusterrole.yaml | 8 ++ .../crds/datadoghq.com_datadogagents_crd.yaml | 6 ++ pkg/apis/datadoghq/v1alpha1/const.go | 2 + .../datadoghq/v1alpha1/datadogagent_types.go | 6 ++ pkg/apis/datadoghq/v1alpha1/test/new.go | 2 + .../v1alpha1/zz_generated.openapi.go | 7 ++ pkg/controller/datadogagent/clusteragent.go | 60 +++++++----- .../datadogagent/clusteragent_test.go | 9 ++ .../datadogagent_controller_test.go | 97 +++++++++++++++++++ 10 files changed, 183 insertions(+), 22 deletions(-) diff --git a/chart/datadog-operator/templates/clusterrole.yaml b/chart/datadog-operator/templates/clusterrole.yaml index 308fb92e7..8d8444d7e 100644 --- a/chart/datadog-operator/templates/clusterrole.yaml +++ b/chart/datadog-operator/templates/clusterrole.yaml @@ -49,4 +49,12 @@ rules: - update - create - delete +- apiGroups: + - "datadoghq.com" + resources: + - "watermarkpodautoscalers" + verbs: + - "list" + - "get" + - "watch" {{- end -}} diff --git a/deploy/clusterrole.yaml b/deploy/clusterrole.yaml index cced83de8..cf825f0cb 100644 --- a/deploy/clusterrole.yaml +++ b/deploy/clusterrole.yaml @@ -62,6 +62,14 @@ rules: - update - create - delete +- apiGroups: + - "datadoghq.com" + resources: + - "watermarkpodautoscalers" + verbs: + - "list" + - "get" + - "watch" --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/crds/datadoghq.com_datadogagents_crd.yaml b/deploy/crds/datadoghq.com_datadogagents_crd.yaml index 214e522a8..eff0715d6 100644 --- a/deploy/crds/datadoghq.com_datadogagents_crd.yaml +++ b/deploy/crds/datadoghq.com_datadogagents_crd.yaml @@ -3499,6 +3499,12 @@ spec: description: Enable usage of DatadogMetrics CRD (allow to scale on arbitrary queries) type: boolean + wpaController: + description: 'Enable informer and controller of the watermark + pod autoscaler NOTE: The WatermarkPodAutoscaler controller + needs to be installed see https://github.com/DataDog/watermarkpodautoscaler + for more details.' + type: boolean type: object logLevel: description: 'Set logging verbosity, valid log levels are: trace, diff --git a/pkg/apis/datadoghq/v1alpha1/const.go b/pkg/apis/datadoghq/v1alpha1/const.go index 7efd21a01..bb5b9c53c 100644 --- a/pkg/apis/datadoghq/v1alpha1/const.go +++ b/pkg/apis/datadoghq/v1alpha1/const.go @@ -67,6 +67,7 @@ const ( DDMetricsProviderEnabled = "DD_EXTERNAL_METRICS_PROVIDER_ENABLED" DDMetricsProviderPort = "DD_EXTERNAL_METRICS_PROVIDER_PORT" DDMetricsProviderUseDatadogMetric = "DD_EXTERNAL_METRICS_PROVIDER_USE_DATADOGMETRIC_CRD" + DDMetricsProviderWPAController = "DD_EXTERNAL_METRICS_PROVIDER_WPA_CONTROLLER" DDAppKey = "DD_APP_KEY" DDClusterChecksEnabled = "DD_CLUSTER_CHECKS_ENABLED" DDClcRunnerEnabled = "DD_CLC_RUNNER_ENABLED" @@ -206,6 +207,7 @@ const ( HorizontalPodAutoscalersRecource = "horizontalpodautoscalers" DatadogMetricsResource = "datadogmetrics" DatadogMetricsStatusResource = "datadogmetrics/status" + WpaResource = "watermarkpodautoscalers" MutatingConfigResource = "mutatingwebhookconfigurations" SecretsResource = "secrets" ReplicasetsResource = "replicasets" diff --git a/pkg/apis/datadoghq/v1alpha1/datadogagent_types.go b/pkg/apis/datadoghq/v1alpha1/datadogagent_types.go index c6f691fae..26f06efd2 100644 --- a/pkg/apis/datadoghq/v1alpha1/datadogagent_types.go +++ b/pkg/apis/datadoghq/v1alpha1/datadogagent_types.go @@ -664,6 +664,12 @@ type ExternalMetricsConfig struct { // +optional Enabled bool `json:"enabled,omitempty"` + // Enable informer and controller of the watermark pod autoscaler + // NOTE: The WatermarkPodAutoscaler controller needs to be installed + // see https://github.com/DataDog/watermarkpodautoscaler for more details. + // +optional + WpaController bool `json:"wpaController,omitempty"` + // Enable usage of DatadogMetrics CRD (allow to scale on arbitrary queries) // +optional UseDatadogMetrics bool `json:"useDatadogMetrics,omitempty"` diff --git a/pkg/apis/datadoghq/v1alpha1/test/new.go b/pkg/apis/datadoghq/v1alpha1/test/new.go index 293371982..ec6549fdf 100644 --- a/pkg/apis/datadoghq/v1alpha1/test/new.go +++ b/pkg/apis/datadoghq/v1alpha1/test/new.go @@ -35,6 +35,7 @@ type NewDatadogAgentOptions struct { MetricsServerEnabled bool MetricsServerPort int32 MetricsServerUseDatadogMetric bool + MetricsServerWPAController bool ClusterChecksEnabled bool NodeAgentConfig *datadoghqv1alpha1.NodeAgentConfig APMEnabled bool @@ -142,6 +143,7 @@ func NewDefaultedDatadogAgent(ns, name string, options *NewDatadogAgentOptions) externalMetricsConfig := datadoghqv1alpha1.ExternalMetricsConfig{ Enabled: true, UseDatadogMetrics: options.MetricsServerUseDatadogMetric, + WpaController: options.MetricsServerWPAController, } if options.MetricsServerPort != 0 { diff --git a/pkg/apis/datadoghq/v1alpha1/zz_generated.openapi.go b/pkg/apis/datadoghq/v1alpha1/zz_generated.openapi.go index 8d759213c..7e266f5d7 100644 --- a/pkg/apis/datadoghq/v1alpha1/zz_generated.openapi.go +++ b/pkg/apis/datadoghq/v1alpha1/zz_generated.openapi.go @@ -1603,6 +1603,13 @@ func schema_pkg_apis_datadoghq_v1alpha1_ExternalMetricsConfig(ref common.Referen Format: "", }, }, + "wpaController": { + SchemaProps: spec.SchemaProps{ + Description: "Enable informer and controller of the watermark pod autoscaler NOTE: The WatermarkPodAutoscaler controller needs to be installed see https://github.com/DataDog/watermarkpodautoscaler for more details.", + Type: []string{"boolean"}, + Format: "", + }, + }, "useDatadogMetrics": { SchemaProps: spec.SchemaProps{ Description: "Enable usage of DatadogMetrics CRD (allow to scale on arbitrary queries)", diff --git a/pkg/controller/datadogagent/clusteragent.go b/pkg/controller/datadogagent/clusteragent.go index d7af06b45..5a9fad1db 100644 --- a/pkg/controller/datadogagent/clusteragent.go +++ b/pkg/controller/datadogagent/clusteragent.go @@ -485,28 +485,33 @@ func getEnvVarsForClusterAgent(dda *datadoghqv1alpha1.DatadogAgent) []corev1.Env Name: datadoghqv1alpha1.DDAPIKey, ValueFrom: getAPIKeyFromSecret(dda), }) - if isMetricsProviderEnabled(spec.ClusterAgent) { - envVars = append(envVars, corev1.EnvVar{ - Name: datadoghqv1alpha1.DDMetricsProviderEnabled, - Value: strconv.FormatBool(spec.ClusterAgent.Config.ExternalMetrics.Enabled), - }) - envVars = append(envVars, corev1.EnvVar{ - Name: datadoghqv1alpha1.DDMetricsProviderPort, - Value: strconv.Itoa(int(getClusterAgentMetricsProviderPort(spec.ClusterAgent.Config))), - }) - envVars = append(envVars, corev1.EnvVar{ - Name: datadoghqv1alpha1.DDAppKey, - ValueFrom: getAppKeyFromSecret(dda), - }) - envVars = append(envVars, corev1.EnvVar{ - Name: datadoghqv1alpha1.DatadogHost, - Value: getDatadogHost(dda), - }) - envVars = append(envVars, corev1.EnvVar{ - Name: datadoghqv1alpha1.DDMetricsProviderUseDatadogMetric, - Value: strconv.FormatBool(spec.ClusterAgent.Config.ExternalMetrics.UseDatadogMetrics), - }) - } + } + + if isMetricsProviderEnabled(spec.ClusterAgent) { + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DDMetricsProviderEnabled, + Value: strconv.FormatBool(spec.ClusterAgent.Config.ExternalMetrics.Enabled), + }) + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DDMetricsProviderPort, + Value: strconv.Itoa(int(getClusterAgentMetricsProviderPort(spec.ClusterAgent.Config))), + }) + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DDAppKey, + ValueFrom: getAppKeyFromSecret(dda), + }) + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DatadogHost, + Value: getDatadogHost(dda), + }) + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DDMetricsProviderUseDatadogMetric, + Value: strconv.FormatBool(spec.ClusterAgent.Config.ExternalMetrics.UseDatadogMetrics), + }) + envVars = append(envVars, corev1.EnvVar{ + Name: datadoghqv1alpha1.DDMetricsProviderWPAController, + Value: strconv.FormatBool(spec.ClusterAgent.Config.ExternalMetrics.WpaController), + }) } // Cluster Checks config @@ -984,6 +989,17 @@ func buildClusterAgentClusterRole(dda *datadoghqv1alpha1.DatadogAgent, name, age Verbs: []string{datadoghqv1alpha1.UpdateVerb}, }) } + + if dda.Spec.ClusterAgent.Config.ExternalMetrics.WpaController { + rbacRules = append(rbacRules, rbacv1.PolicyRule{ + APIGroups: []string{datadoghqv1alpha1.DatadogAPIGroup}, + Resources: []string{datadoghqv1alpha1.WpaResource}, + Verbs: []string{ + datadoghqv1alpha1.ListVerb, + datadoghqv1alpha1.WatchVerb, + datadoghqv1alpha1.GetVerb}, + }) + } } if dda.Spec.ClusterAgent.Config.AdmissionController != nil && dda.Spec.ClusterAgent.Config.AdmissionController.Enabled { diff --git a/pkg/controller/datadogagent/clusteragent_test.go b/pkg/controller/datadogagent/clusteragent_test.go index 1efb3410c..f304132a3 100644 --- a/pkg/controller/datadogagent/clusteragent_test.go +++ b/pkg/controller/datadogagent/clusteragent_test.go @@ -487,6 +487,10 @@ func Test_newClusterAgentDeploymentFromInstance_MetricsServer(t *testing.T) { Name: datadoghqv1alpha1.DDMetricsProviderUseDatadogMetric, Value: "false", }, + { + Name: datadoghqv1alpha1.DDMetricsProviderWPAController, + Value: "false", + }, }..., ) @@ -543,6 +547,10 @@ func Test_newClusterAgentDeploymentFromInstance_MetricsServer(t *testing.T) { Name: datadoghqv1alpha1.DDMetricsProviderUseDatadogMetric, Value: "true", }, + { + Name: datadoghqv1alpha1.DDMetricsProviderWPAController, + Value: "true", + }, }..., ) metricsServerWithSitePodSpec.Containers[0].LivenessProbe = probe @@ -560,6 +568,7 @@ func Test_newClusterAgentDeploymentFromInstance_MetricsServer(t *testing.T) { ClusterAgentEnabled: true, MetricsServerEnabled: true, MetricsServerUseDatadogMetric: true, + MetricsServerWPAController: true, Site: "datadoghq.eu", MetricsServerPort: metricsServerPort, }) diff --git a/pkg/controller/datadogagent/datadogagent_controller_test.go b/pkg/controller/datadogagent/datadogagent_controller_test.go index 29df4e15e..6badeb1eb 100644 --- a/pkg/controller/datadogagent/datadogagent_controller_test.go +++ b/pkg/controller/datadogagent/datadogagent_controller_test.go @@ -9,6 +9,7 @@ import ( "context" "encoding/base64" "errors" + "reflect" "fmt" "testing" @@ -1146,6 +1147,67 @@ func TestReconcileDatadogAgent_Reconcile(t *testing.T) { return nil }, }, + { + name: "DatadogAgent found and defaulted, Cluster Agent enabled, WPA Controller enabled, create the Cluster Agent ClusterRole", + fields: fields{ + client: fake.NewFakeClient(), + scheme: s, + recorder: recorder, + }, + args: args{ + request: newRequest(resourcesNamespace, resourcesName), + loadFunc: func(c client.Client) { + dda := test.NewDefaultedDatadogAgent(resourcesNamespace, resourcesName, &test.NewDatadogAgentOptions{Labels: map[string]string{"label-foo-key": "label-bar-value"}, ClusterAgentEnabled: true, MetricsServerEnabled: true, MetricsServerWPAController: true}) + _ = c.Create(context.TODO(), dda) + commonDCAlabels := getDefaultLabels(dda, datadoghqv1alpha1.DefaultClusterAgentResourceSuffix, getClusterAgentVersion(dda)) + _ = c.Create(context.TODO(), test.NewSecret(resourcesNamespace, "foo", &test.NewSecretOptions{Labels: commonDCAlabels, Data: map[string][]byte{ + "token": []byte(base64.StdEncoding.EncodeToString([]byte("token-foo"))), + }})) + + createClusterAgentDependencies(c, dda) + + dcaExternalMetricsService := test.NewService(resourcesNamespace, "foo-cluster-agent-metrics-server", &test.NewServiceOptions{Spec: &corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{ + datadoghqv1alpha1.AgentDeploymentNameLabelKey: resourcesName, + datadoghqv1alpha1.AgentDeploymentComponentLabelKey: "cluster-agent", + }, + Ports: []corev1.ServicePort{ + { + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt(datadoghqv1alpha1.DefaultMetricsServerTargetPort), + Port: datadoghqv1alpha1.DefaultMetricsServerServicePort, + }, + }, + SessionAffinity: corev1.ServiceAffinityNone, + }, + }) + _, _ = comparison.SetMD5GenerationAnnotation(&dcaExternalMetricsService.ObjectMeta, dcaExternalMetricsService.Spec) + dcaExternalMetricsService.Labels = commonDCAlabels + _ = c.Create(context.TODO(), dcaExternalMetricsService) + _ = c.Create(context.TODO(), buildClusterAgentPDB(dda)) + }, + }, + want: reconcile.Result{Requeue: true}, + wantErr: false, + wantFunc: func(c client.Client) error { + metricsService := &corev1.Service{} + if err := c.Get(context.TODO(), newRequest(resourcesNamespace, "foo-cluster-agent-metrics-server").NamespacedName, metricsService); err != nil { + return err + } + clusterRole := &rbacv1.ClusterRole{} + if err := c.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesNameClusterAgent}, clusterRole); err != nil { + return err + } + if !hasAllClusterLevelRbacResources(clusterRole.Rules) { + return fmt.Errorf("bad cluster role, should contain all cluster level rbac resources, current: %v", clusterRole.Rules) + } + if !hasWpaRbacs(clusterRole.Rules) { + return fmt.Errorf("bad cluster role, should contain wpa cluster level rbac resources, current: %v", clusterRole.Rules) + } + return nil + }, + }, { name: "DatadogAgent found and defaulted, Cluster Agent enabled, Admission Controller enabled, create the Cluster Agent ClusterRole", fields: fields{ @@ -2103,6 +2165,41 @@ func hasAllClusterLevelRbacResources(policyRules []rbacv1.PolicyRule) bool { return len(clusterLevelResources) == 0 } +func hasWpaRbacs(policyRules []rbacv1.PolicyRule) bool { + requiredVerbs := []string{ + datadoghqv1alpha1.ListVerb, + datadoghqv1alpha1.WatchVerb, + datadoghqv1alpha1.GetVerb, + } + + for _, policyRule := range policyRules { + resourceFound := false + groupFound := false + verbsFound := false + + for _, resource := range policyRule.Resources { + if resource == "watermarkpodautoscalers" { + resourceFound = true + break + } + } + for _, group := range policyRule.APIGroups { + if group == "datadoghq.com" { + groupFound = true + break + } + } + if reflect.DeepEqual(policyRule.Verbs, requiredVerbs) { + verbsFound = true + } + if resourceFound && groupFound && verbsFound { + return true + } + } + + return false +} + func hasAdmissionRbacResources(policyRules []rbacv1.PolicyRule) bool { clusterLevelResources := map[string]bool{ "secrets": true,