From b7b0c174160d043896c0795853a2880e5c8163cc Mon Sep 17 00:00:00 2001 From: Bharathi Tenneti Date: Tue, 21 Jul 2020 15:32:42 -0400 Subject: [PATCH] Remove metrics logic from cmd/anisble-operator/main.go (#3466) * Remove ansible operator metrics logic --- changelog/fragments/rm-ansible-metrics.yml | 11 +++ cmd/ansible-operator/main.go | 83 +----------------- hack/tests/e2e-ansible.sh | 60 ++++++------- internal/scaffold/role.go | 28 ------- internal/scaffold/role_test.go | 84 ------------------- .../cluster/tasks/servicemonitor_test.yml | 4 - 6 files changed, 39 insertions(+), 231 deletions(-) create mode 100644 changelog/fragments/rm-ansible-metrics.yml delete mode 100644 test/ansible/molecule/cluster/tasks/servicemonitor_test.yml diff --git a/changelog/fragments/rm-ansible-metrics.yml b/changelog/fragments/rm-ansible-metrics.yml new file mode 100644 index 00000000000..59cf0ecacd5 --- /dev/null +++ b/changelog/fragments/rm-ansible-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/ansible-operator/main.go, and tests/e2e-anisble.sh checks for servicemonitor. + body: TBD \ No newline at end of file diff --git a/cmd/ansible-operator/main.go b/cmd/ansible-operator/main.go index 462f3eb9a78..30a6565106f 100644 --- a/cmd/ansible-operator/main.go +++ b/cmd/ansible-operator/main.go @@ -16,7 +16,6 @@ package main import ( "context" - "errors" "fmt" "os" "runtime" @@ -24,10 +23,7 @@ import ( "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" "sigs.k8s.io/controller-runtime/pkg/client" @@ -44,19 +40,16 @@ import ( "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap" "github.com/operator-framework/operator-sdk/pkg/ansible/runner" "github.com/operator-framework/operator-sdk/pkg/ansible/watches" - kubemetrics "github.com/operator-framework/operator-sdk/pkg/kube-metrics" "github.com/operator-framework/operator-sdk/pkg/leader" "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" ) var ( - metricsHost = "0.0.0.0" - log = logf.Log.WithName("cmd") - metricsPort int32 = 8383 - operatorMetricsPort int32 = 8686 - healthProbePort int32 = 6789 + metricsHost = "0.0.0.0" + log = logf.Log.WithName("cmd") + metricsPort int32 = 8383 + healthProbePort int32 = 6789 ) func printVersion() { @@ -125,7 +118,6 @@ func main() { os.Exit(1) } - var gvks []schema.GroupVersionKind cMap := controllermap.NewControllerMap() watches, err := watches.Load(f.WatchesFile, f.MaxConcurrentReconciles, f.AnsibleVerbosity) if err != nil { @@ -159,7 +151,6 @@ func main() { OwnerWatchMap: controllermap.NewWatchMap(), AnnotationWatchMap: controllermap.NewWatchMap(), }, w.Blacklist) - gvks = append(gvks, w.GroupVersionKind) } operatorName, err := k8sutil.GetOperatorName() @@ -175,7 +166,6 @@ func main() { os.Exit(1) } - addMetrics(context.TODO(), cfg, gvks) err = mgr.AddHealthzCheck("ping", healthz.Ping) if err != nil { log.Error(err, "Failed to add Healthz check.") @@ -213,71 +203,6 @@ func main() { log.Info("Exiting.") } -// 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: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, - TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}}, - {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()) - return - } - - // 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 takes GVKs retrieved from watches 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 -} - // getAnsibleDebugLog return the value from the ANSIBLE_DEBUG_LOGS it order to // print the full Ansible logs func getAnsibleDebugLog() bool { diff --git a/hack/tests/e2e-ansible.sh b/hack/tests/e2e-ansible.sh index 0c5fddc1887..617f86b57c2 100755 --- a/hack/tests/e2e-ansible.sh +++ b/hack/tests/e2e-ansible.sh @@ -59,37 +59,26 @@ test_operator() { exit 1 fi - header_text "verify that metrics service was created" - if ! timeout 60s bash -c -- "until kubectl get service/memcached-operator-metrics > /dev/null 2>&1; do sleep 1; done"; - then - error_text "FAIL: Failed to get metrics service" - operator_logs - exit 1 - fi + # TODO @asmacdo to uncomment once new kb layout is merged. - header_text "verify that the metrics endpoint exists (Port 8383)" - if ! timeout 1m bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sfo /dev/null http://memcached-operator-metrics:8383/metrics; do sleep 1; done"; - then - error_text "FAIL: Failed to verify that metrics endpoint exists" - operator_logs - exit 1 - fi + # header_text "verify that metrics service was created" + # if ! timeout 60s bash -c -- "until kubectl get service/memcached-operator-metrics > /dev/null 2>&1; do sleep 1; done"; + # then + # error_text "FAIL: Failed to get metrics service" + # operator_logs + # exit 1 + # fi - header_text "verify that the metrics endpoint exists (Port 8686)" - if ! timeout 1m bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sfo /dev/null http://memcached-operator-metrics:8686/metrics; do sleep 1; done"; - then - error_text "FAIL: Failed to verify that metrics endpoint exists" - operator_logs - exit 1 - fi + # TODO Add --metrics-addr flag to the ansible operator and default it to 8080. + + # header_text "verify that the metrics endpoint exists (Port 8383)" + # if ! timeout 1m bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sfo /dev/null http://memcached-operator-metrics:8383/metrics; do sleep 1; done"; + # then + # error_text "FAIL: Failed to verify that metrics endpoint exists" + # operator_logs + # exit 1 + # fi - header_text "verify that the servicemonitor is created" - if ! timeout 1m bash -c -- "until kubectl get servicemonitors/memcached-operator-metrics > /dev/null 2>&1; do sleep 1; done"; - then - error_text "FAIL: Failed to get service monitor" - operator_logs - exit 1 - fi header_text "create custom resource (Memcached CR)" kubectl create -f deploy/crds/ansible.example.com_v1alpha1_memcached_cr.yaml @@ -136,13 +125,13 @@ test_operator() { fi - header_text "verify that metrics reflect cr creation" - if ! timeout 60s bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sf http://memcached-operator-metrics:8686/metrics | grep example-memcached; do sleep 1; done"; - then - error_text "FAIL: Failed to verify custom resource metrics" - operator_logs - exit 1 - fi + # header_text "verify that metrics reflect cr creation" + # if ! timeout 60s bash -c -- "until kubectl run --attach --rm --restart=Never test-metrics --image=$metrics_test_image -- curl -sf http://memcached-operator-metrics:8383/metrics | grep example-memcached; do sleep 1; done"; + # then + # error_text "FAIL: Failed to verify custom resource metrics" + # operator_logs + # exit 1 + # fi header_text "get memcached deploy by labels" memcached_deployment=$(kubectl get deployment -l app=memcached -o jsonpath="{..metadata.name}") @@ -197,7 +186,6 @@ cat "$ROOTDIR/test/ansible-memcached/watches-finalizer.yaml" >> memcached-operat # Append Foo kind to watches to test watching multiple Kinds cat "$ROOTDIR/test/ansible-memcached/watches-foo-kind.yaml" >> memcached-operator/watches.yaml -install_service_monitor_crd pushd memcached-operator diff --git a/internal/scaffold/role.go b/internal/scaffold/role.go index a785fa6b079..595c6aae833 100644 --- a/internal/scaffold/role.go +++ b/internal/scaffold/role.go @@ -326,32 +326,4 @@ rules: {{- end }} {{- end }} {{- end }} -- apiGroups: - - monitoring.coreos.com - resources: - - servicemonitors - verbs: - - "get" - - "create" -- apiGroups: - - apps - resources: - - deployments/finalizers - resourceNames: - - {{ .ProjectName }} - verbs: - - "update" -- apiGroups: - - "" - resources: - - pods - verbs: - - get -- apiGroups: - - apps - resources: - - replicasets - - deployments - verbs: - - get ` diff --git a/internal/scaffold/role_test.go b/internal/scaffold/role_test.go index e86d4ce2f87..d58204159f6 100644 --- a/internal/scaffold/role_test.go +++ b/internal/scaffold/role_test.go @@ -120,34 +120,6 @@ rules: - patch - update - watch -- apiGroups: - - monitoring.coreos.com - resources: - - servicemonitors - verbs: - - "get" - - "create" -- apiGroups: - - apps - resources: - - deployments/finalizers - resourceNames: - - app-operator - verbs: - - "update" -- apiGroups: - - "" - resources: - - pods - verbs: - - get -- apiGroups: - - apps - resources: - - replicasets - - deployments - verbs: - - get ` const clusterroleExp = `kind: ClusterRole @@ -189,34 +161,6 @@ rules: - patch - update - watch -- apiGroups: - - monitoring.coreos.com - resources: - - servicemonitors - verbs: - - "get" - - "create" -- apiGroups: - - apps - resources: - - deployments/finalizers - resourceNames: - - app-operator - verbs: - - "update" -- apiGroups: - - "" - resources: - - pods - verbs: - - get -- apiGroups: - - apps - resources: - - replicasets - - deployments - verbs: - - get ` const roleCustomRulesExp = `kind: Role @@ -245,32 +189,4 @@ rules: resources: - "roles" - "rolebindings" -- apiGroups: - - monitoring.coreos.com - resources: - - servicemonitors - verbs: - - "get" - - "create" -- apiGroups: - - apps - resources: - - deployments/finalizers - resourceNames: - - app-operator - verbs: - - "update" -- apiGroups: - - "" - resources: - - pods - verbs: - - get -- apiGroups: - - apps - resources: - - replicasets - - deployments - verbs: - - get ` diff --git a/test/ansible/molecule/cluster/tasks/servicemonitor_test.yml b/test/ansible/molecule/cluster/tasks/servicemonitor_test.yml deleted file mode 100644 index 8768c464645..00000000000 --- a/test/ansible/molecule/cluster/tasks/servicemonitor_test.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -- name: Assert ServiceMonitor resource is created - assert: - that: lookup('k8s', kind='ServiceMonitor', api_version='monitoring.coreos.com/v1', namespace=namespace, resource_name='ansible-metrics')