From 5144f4b6777aa8e8e82d21f57262dd32c679b294 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 09:47:42 -0400 Subject: [PATCH 1/7] Remove metrics logic from cmd/helm-operator/main.go --- cmd/helm-operator/main.go | 70 --------------------------------------- 1 file changed, 70 deletions(-) diff --git a/cmd/helm-operator/main.go b/cmd/helm-operator/main.go index 131d04859e..fd197cad53 100644 --- a/cmd/helm-operator/main.go +++ b/cmd/helm-operator/main.go @@ -15,18 +15,14 @@ package main import ( - "context" - "errors" "fmt" "os" "runtime" "strings" "github.com/spf13/pflag" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/cache" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -40,9 +36,7 @@ import ( "github.com/operator-framework/operator-sdk/pkg/helm/release" "github.com/operator-framework/operator-sdk/pkg/helm/watches" "github.com/operator-framework/operator-sdk/pkg/k8sutil" - kubemetrics "github.com/operator-framework/operator-sdk/pkg/kube-metrics" "github.com/operator-framework/operator-sdk/pkg/log/zap" - "github.com/operator-framework/operator-sdk/pkg/metrics" sdkVersion "github.com/operator-framework/operator-sdk/version" ) @@ -154,73 +148,9 @@ func main() { gvks = append(gvks, w.GroupVersionKind) } - addMetrics(context.TODO(), cfg, gvks) - // Start the Cmd if err = mgr.Start(signals.SetupSignalHandler()); err != nil { log.Error(err, "Manager exited non-zero.") os.Exit(1) } } - -// addMetrics will create the Services and Service Monitors to allow the operator export the metrics by using -// the Prometheus operator -func addMetrics(ctx context.Context, cfg *rest.Config, gvks []schema.GroupVersionKind) { - // Get the namespace the operator is currently deployed in. - operatorNs, err := k8sutil.GetOperatorNamespace() - if err != nil { - if errors.Is(err, k8sutil.ErrRunLocal) { - log.Info("Skipping CR metrics server creation; not running in a cluster.") - return - } - } - - if err := serveCRMetrics(cfg, operatorNs, gvks); err != nil { - log.Info("Could not generate and serve custom resource metrics", "error", err.Error()) - } - - // Add to the below struct any other metrics ports you want to expose. - servicePorts := []v1.ServicePort{ - {Port: operatorMetricsPort, Name: metrics.CRPortName, Protocol: v1.ProtocolTCP, - TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort}}, - } - - // Create Service object to expose the metrics port(s). - service, err := metrics.CreateMetricsService(ctx, cfg, servicePorts) - if err != nil { - log.Info("Could not create metrics Service", "error", err.Error()) - } - - // CreateServiceMonitors will automatically create the prometheus-operator ServiceMonitor resources - // necessary to configure Prometheus to scrape metrics from this operator. - services := []*v1.Service{service} - - // The ServiceMonitor is created in the same namespace where the operator is deployed - _, err = metrics.CreateServiceMonitors(cfg, operatorNs, services) - if err != nil { - log.Info("Could not create ServiceMonitor object", "error", err.Error()) - // If this operator is deployed to a cluster without the prometheus-operator running, it will return - // ErrServiceMonitorNotPresent, which can be used to safely skip ServiceMonitor creation. - if err == metrics.ErrServiceMonitorNotPresent { - log.Info("Install prometheus-operator in your cluster to create ServiceMonitor objects", "error", err.Error()) - } - } -} - -// serveCRMetrics gets the Operator/CustomResource GVKs and generates metrics based on those types. -// It serves those metrics on "http://metricsHost:operatorMetricsPort". -func serveCRMetrics(cfg *rest.Config, operatorNs string, gvks []schema.GroupVersionKind) error { - // The metrics will be generated from the namespaces which are returned here. - // NOTE that passing nil or an empty list of namespaces in GenerateAndServeCRMetrics will result in an error. - ns, err := kubemetrics.GetNamespacesForMetrics(operatorNs) - if err != nil { - return err - } - - // Generate and serve custom resource specific metrics. - err = kubemetrics.GenerateAndServeCRMetrics(cfg, ns, gvks, metricsHost, operatorMetricsPort) - if err != nil { - return err - } - return nil -} From bcc99f21c3e352fafd6c5123920ef65d33713123 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 10:24:18 -0400 Subject: [PATCH 2/7] Added changelog fragments, fixing sanity checks --- cmd/helm-operator/main.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/cmd/helm-operator/main.go b/cmd/helm-operator/main.go index fd197cad53..65cbb56479 100644 --- a/cmd/helm-operator/main.go +++ b/cmd/helm-operator/main.go @@ -22,7 +22,6 @@ import ( "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/cache" crclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -40,12 +39,7 @@ import ( sdkVersion "github.com/operator-framework/operator-sdk/version" ) -var ( - metricsHost = "0.0.0.0" - operatorMetricsPort int32 = 8686 - - log = logf.Log.WithName("cmd") -) +var log = logf.Log.WithName("cmd") func printVersion() { log.Info(fmt.Sprintf("Go Version: %s", runtime.Version())) @@ -129,7 +123,6 @@ func main() { log.Error(err, "Failed to create new manager factories.") os.Exit(1) } - var gvks []schema.GroupVersionKind for _, w := range ws { // Register the controller with the factory. err := controller.Add(mgr, controller.WatchOptions{ @@ -145,7 +138,6 @@ func main() { log.Error(err, "Failed to add manager factory to controller.") os.Exit(1) } - gvks = append(gvks, w.GroupVersionKind) } // Start the Cmd From d5dc739083db382fcc0364b333c39cda042ced63 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 10:51:49 -0400 Subject: [PATCH 3/7] Remove checks for servicemonitor in test-e2e-helm.sh --- hack/tests/e2e-helm.sh | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/hack/tests/e2e-helm.sh b/hack/tests/e2e-helm.sh index 215b06f14d..9b8c038309 100755 --- a/hack/tests/e2e-helm.sh +++ b/hack/tests/e2e-helm.sh @@ -14,8 +14,6 @@ setup_envs $tmp_sdk_root # kind has an issue with certain image registries (ex. redhat's), so use a # different test pod image. -METRICS_TEST_IMAGE="curlimages/curl:latest" - test_namespace="nginx-cr-system" operator_namespace="nginx-operator-system" @@ -42,7 +40,6 @@ operator_logs() { test_operator() { # kind has an issue with certain image registries (ex. redhat's), so use a # different test pod image. - local metrics_test_image="$METRICS_TEST_IMAGE" # wait for operator pod to run if ! timeout 1m kubectl rollout status deployment/nginx-operator-controller-manager -n ${operator_namespace} ; @@ -52,28 +49,6 @@ test_operator() { exit 1 fi - metrics_service="nginx-operator-controller-manager-metrics-service" - - # verify that metrics service was created - if ! timeout 60s bash -c -- "until kubectl get service/${metrics_service} --namespace=${operator_namespace} > /dev/null 2>&1; do sleep 1; done"; - then - error_text "Failed to get metrics service" - operator_logs - exit 1 - fi - - - serviceaccount_secret=$(kubectl get serviceaccounts default -n ${operator_namespace} -o jsonpath='{.secrets[0].name}') - token=$(kubectl get secret ${serviceaccount_secret} -n ${operator_namespace} -o jsonpath={.data.token} | base64 -d) - - # verify that the metrics endpoint exists - if ! timeout 60s bash -c -- "until kubectl run --attach --rm --restart=Never --namespace=${operator_namespace} test-metrics --image=${metrics_test_image} -- -sfkH \"Authorization: Bearer ${token}\" https://${metrics_service}:8443/metrics; do sleep 1; done"; - then - error_text "Failed to verify that metrics endpoint exists" - operator_logs - exit 1 - fi - # create CR kubectl create --namespace=${test_namespace} -f config/samples/helm.example_v1alpha1_nginx.yaml trap_add "kubectl delete --namespace=${test_namespace} --ignore-not-found -f ${OPERATORDIR}/config/samples/helm.example_v1alpha1_nginx.yaml" EXIT @@ -84,13 +59,6 @@ test_operator() { exit 1 fi - header_text "verify that the servicemonitor is created" - if ! timeout 1m bash -c -- "until kubectl get servicemonitors/nginx-operator-metrics --namespace=${operator_namespace} > /dev/null 2>&1; do sleep 1; done"; - then - error_text "FAIL: Failed to get service monitor" - operator_logs - exit 1 - fi release_name=$(kubectl get --namespace=${test_namespace} nginxes.helm.example.com nginx-sample -o jsonpath="{..status.deployedRelease.name}") nginx_deployment=$(kubectl get --namespace=${test_namespace} deployment -l "app.kubernetes.io/instance=${release_name}" -o jsonpath="{..metadata.name}") @@ -145,7 +113,6 @@ if echo $log | grep -q "failed to generate RBAC rules"; then exit 1 fi -install_service_monitor_crd sed -i".bak" -E -e 's/(FROM quay.io\/operator-framework\/helm-operator)(:.*)?/\1:dev/g' Dockerfile; rm -f Dockerfile.bak make docker-build IMG="$DEST_IMAGE" @@ -153,9 +120,7 @@ make docker-build IMG="$DEST_IMAGE" # If using a kind cluster, load the image into all nodes. load_image_if_kind "$DEST_IMAGE" -docker pull "$METRICS_TEST_IMAGE" -# If using a kind cluster, load the metrics test image into all nodes. -load_image_if_kind "$METRICS_TEST_IMAGE" + OPERATORDIR="$(pwd)" From 8e9385c81b1056576cb2bd751bac39ae96444c6b Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 10:57:15 -0400 Subject: [PATCH 4/7] Added changelog fragment --- changelog/fragments/rm-helm-metrics.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog/fragments/rm-helm-metrics.yml diff --git a/changelog/fragments/rm-helm-metrics.yml b/changelog/fragments/rm-helm-metrics.yml new file mode 100644 index 0000000000..539d0f68e4 --- /dev/null +++ b/changelog/fragments/rm-helm-metrics.yml @@ -0,0 +1,11 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + Remove legacy metrics generation code. + kind: "removal" + # Is this a breaking change? + breaking: true + migration: + header: Remove legacy metrics generation code from cmd/helm-operator/main.go, and test-e2e-helm.sh checks for servicemonitor. + body: TBD \ No newline at end of file From e31105be291ec56274ca770b395cbcf503fd020f Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 11:11:59 -0400 Subject: [PATCH 5/7] Removed clusterrolBinding for metric service, removed env section from podspec template from helm plugin scaffold --- hack/tests/e2e-helm.sh | 2 -- .../helm/v1/scaffolds/templates/manager/config.go | 9 --------- 2 files changed, 11 deletions(-) diff --git a/hack/tests/e2e-helm.sh b/hack/tests/e2e-helm.sh index 9b8c038309..6d4e45b77c 100755 --- a/hack/tests/e2e-helm.sh +++ b/hack/tests/e2e-helm.sh @@ -20,14 +20,12 @@ operator_namespace="nginx-operator-system" deploy_operator() { make install make deploy IMG="$DEST_IMAGE" - kubectl create clusterrolebinding nginx-operator-system-metrics-reader --clusterrole=nginx-operator-metrics-reader --serviceaccount=${operator_namespace}:default kubectl create namespace ${test_namespace} } remove_operator() { kubectl delete --ignore-not-found=true --namespace=${test_namespace} -f "$OPERATORDIR/config/samples/helm.example_v1alpha1_nginx.yaml" kubectl delete --ignore-not-found=true namespace ${test_namespace} - kubectl delete --ignore-not-found=true clusterrolebinding nginx-operator-system-metrics-reader make undeploy } diff --git a/internal/plugins/helm/v1/scaffolds/templates/manager/config.go b/internal/plugins/helm/v1/scaffolds/templates/manager/config.go index 4690238097..304492a7a9 100644 --- a/internal/plugins/helm/v1/scaffolds/templates/manager/config.go +++ b/internal/plugins/helm/v1/scaffolds/templates/manager/config.go @@ -93,14 +93,5 @@ spec: requests: cpu: 100m memory: 60Mi - env: - - name: WATCH_NAMESPACE - value: "" - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - - name: OPERATOR_NAME - value: "{{ .OperatorName }}" terminationGracePeriodSeconds: 10 ` From 33c2b2f1cffc0ab6cb7c5e87a77451cc4a7914e2 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 12:32:56 -0400 Subject: [PATCH 6/7] reverted servcie creation --- hack/tests/e2e-helm.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/hack/tests/e2e-helm.sh b/hack/tests/e2e-helm.sh index 6d4e45b77c..ad15953004 100755 --- a/hack/tests/e2e-helm.sh +++ b/hack/tests/e2e-helm.sh @@ -14,6 +14,8 @@ setup_envs $tmp_sdk_root # kind has an issue with certain image registries (ex. redhat's), so use a # different test pod image. +METRICS_TEST_IMAGE="curlimages/curl:latest" + test_namespace="nginx-cr-system" operator_namespace="nginx-operator-system" @@ -38,6 +40,7 @@ operator_logs() { test_operator() { # kind has an issue with certain image registries (ex. redhat's), so use a # different test pod image. + local metrics_test_image="$METRICS_TEST_IMAGE" # wait for operator pod to run if ! timeout 1m kubectl rollout status deployment/nginx-operator-controller-manager -n ${operator_namespace} ; @@ -47,6 +50,28 @@ test_operator() { exit 1 fi + metrics_service="nginx-operator-controller-manager-metrics-service" + + # verify that metrics service was created + if ! timeout 60s bash -c -- "until kubectl get service/${metrics_service} --namespace=${operator_namespace} > /dev/null 2>&1; do sleep 1; done"; + then + error_text "Failed to get metrics service" + operator_logs + exit 1 + fi + + + serviceaccount_secret=$(kubectl get serviceaccounts default -n ${operator_namespace} -o jsonpath='{.secrets[0].name}') + token=$(kubectl get secret ${serviceaccount_secret} -n ${operator_namespace} -o jsonpath={.data.token} | base64 -d) + + # verify that the metrics endpoint exists + if ! timeout 60s bash -c -- "until kubectl run --attach --rm --restart=Never --namespace=${operator_namespace} test-metrics --image=${metrics_test_image} -- -sfkH \"Authorization: Bearer ${token}\" https://${metrics_service}:8443/metrics; do sleep 1; done"; + then + error_text "Failed to verify that metrics endpoint exists" + operator_logs + exit 1 + fi + # create CR kubectl create --namespace=${test_namespace} -f config/samples/helm.example_v1alpha1_nginx.yaml trap_add "kubectl delete --namespace=${test_namespace} --ignore-not-found -f ${OPERATORDIR}/config/samples/helm.example_v1alpha1_nginx.yaml" EXIT From f3d9da910e176e96d57099d801889959096b2574 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Fri, 17 Jul 2020 13:12:56 -0400 Subject: [PATCH 7/7] reverted clusterrolebinding metrics --- hack/tests/e2e-helm.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hack/tests/e2e-helm.sh b/hack/tests/e2e-helm.sh index ad15953004..71c9d83a4d 100755 --- a/hack/tests/e2e-helm.sh +++ b/hack/tests/e2e-helm.sh @@ -22,12 +22,14 @@ operator_namespace="nginx-operator-system" deploy_operator() { make install make deploy IMG="$DEST_IMAGE" + kubectl create clusterrolebinding nginx-operator-system-metrics-reader --clusterrole=nginx-operator-metrics-reader --serviceaccount=${operator_namespace}:default kubectl create namespace ${test_namespace} } remove_operator() { kubectl delete --ignore-not-found=true --namespace=${test_namespace} -f "$OPERATORDIR/config/samples/helm.example_v1alpha1_nginx.yaml" kubectl delete --ignore-not-found=true namespace ${test_namespace} + kubectl delete --ignore-not-found=true clusterrolebinding nginx-operator-system-metrics-reader make undeploy } @@ -83,6 +85,7 @@ test_operator() { fi + release_name=$(kubectl get --namespace=${test_namespace} nginxes.helm.example.com nginx-sample -o jsonpath="{..status.deployedRelease.name}") nginx_deployment=$(kubectl get --namespace=${test_namespace} deployment -l "app.kubernetes.io/instance=${release_name}" -o jsonpath="{..metadata.name}") @@ -143,10 +146,12 @@ make docker-build IMG="$DEST_IMAGE" # If using a kind cluster, load the image into all nodes. load_image_if_kind "$DEST_IMAGE" - +docker pull "$METRICS_TEST_IMAGE" +# If using a kind cluster, load the metrics test image into all nodes. +load_image_if_kind "$METRICS_TEST_IMAGE" OPERATORDIR="$(pwd)" deploy_operator trap_add 'remove_operator' EXIT -test_operator +test_operator \ No newline at end of file