Vmalert notifier fixes (#130)

* fixes vmalert panic,
fixes tlsConfig build for vmalert,
added testcases for vmalert tlsConfig

* fixes vmnodescape for operator-hub integration

api/v1beta1/vmnodescrape_types.go

@@ -7,9 +7,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// VMNodeScrapeSpec defines discovery for targets placed on kubernetes nodes,
-// usually its node-exporters and other host services.
-// InternalIP is used as __address__ for scraping.
+// VMNodeScrapeSpec defines specification for VMNodeScrape.
 type VMNodeScrapeSpec struct {
 	// The label to use to retrieve the job name from.
 	// +optional
@@ -84,7 +82,9 @@ type VMNodeScrapeStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 
-// VMNodeScrape is the Schema for the vmnodescrapes API
+// VMNodeScrape defines discovery for targets placed on kubernetes nodes,
+// usually its node-exporters and other host services.
+// InternalIP is used as __address__ for scraping.
 type VMNodeScrape struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

config/crd/bases/operator.victoriametrics.com_vmnodescrapes.yaml

@@ -19,7 +19,9 @@ spec:
     status: {}
   validation:
     openAPIV3Schema:
-      description: VMNodeScrape is the Schema for the vmnodescrapes API
+      description: VMNodeScrape defines discovery for targets placed on kubernetes
+        nodes, usually its node-exporters and other host services. InternalIP is used
+        as __address__ for scraping.
       properties:
         apiVersion:
           description: 'APIVersion defines the versioned schema of this representation
@@ -34,9 +36,7 @@ spec:
         metadata:
           type: object
         spec:
-          description: VMNodeScrapeSpec defines discovery for targets placed on kubernetes
-            nodes, usually its node-exporters and other host services. InternalIP
-            is used as __address__ for scraping.
+          description: VMNodeScrapeSpec defines specification for VMNodeScrape.
           properties:
             basicAuth:
               description: 'BasicAuth allow an endpoint to authenticate over basic

config/manifests/bases/victoriametrics-operator.clusterserviceversion.yaml

@@ -117,7 +117,7 @@ spec:
         path: resources
         x-descriptors:
         - urn:alm:descriptor:com.tectonic.ui:resourceRequirements
-      - description: ServiceAccountName is the name of the ServiceAccount to use to run the VMAgent Pods. required
+      - description: ServiceAccountName is the name of the ServiceAccount to use to run the VMAgent Pods.
         displayName: ServiceAccount name
         path: serviceAccountName
         x-descriptors:
@@ -164,6 +164,11 @@ spec:
         x-descriptors:
         - urn:alm:descriptor:com.tectonic.ui:resourceRequirements
       version: v1beta1
+    - description: VMNodeScrape is the Schema for the vmnodescrapes API
+      displayName: VMNode Scrape
+      kind: VMNodeScrape
+      name: vmnodescrapes.operator.victoriametrics.com
+      version: v1beta1
   description: |
     Operator manages VictoriaMetrics applications and provides monitoring features for applications running inside and outside kubernetes cluster. It has support for prometheus-operator objects and
     provides migration mechanism.

config/samples/kustomization.yaml

@@ -8,4 +8,5 @@ resources:
 - victoriametrics_v1beta1_vmservicescrape.yaml
 - victoriametrics_v1beta1_vmsingle.yaml
 - victoriametrics_v1beta1_vmcluster.yaml
-- operator_v1beta1_vmprobe.yaml
+- operator_v1beta1_vmprobe.yaml
+- operator_v1beta1_vmnodescrape.yaml

config/samples/operator_v1beta1_vmnodescrape.yaml

@@ -11,31 +11,12 @@ spec:
   selector:
     matchLabels:
       role: node-exporter
-  params:
-    urlParam: ["value1", "value3"]
   interval: "10s"
   scrapeTimeout: "2s"
   tlsConfig:
     insecureSkipVerify: true
-    serverName: "server"
-#    keyFile: "/tmp/key.pem"
-#    certFile: "/tmp/cert.pem"
     caFile: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
   bearerTokenFile: "/var/run/secrets/kubernetes.io/serviceaccount/token"
-#  bearerTokenSecret:
-#    name: "bearer-secret"
-#    key: "bearer"
-  metricRelabelConfigs:
-    - sourceLabels: ["node"]
-      targetLabel: "kubernetes-node"
   relabelConfigs:
     - sourceLabels: [ "node" ]
       targetLabel: "kubernetes_io_node1"
-#  basicAuth:
-#    username:
-#      name: "secret-1"
-#      key: "password"
-#    password:
-#      name: "secret-1"
-#      key: "password"
-  sampleLimit: 100000

controllers/factory/scrapes.go

@@ -264,31 +264,29 @@ func SelectPodScrapes(ctx context.Context, cr *victoriametricsv1beta1.VMAgent, r
 
 	podScrapesCombined := []victoriametricsv1beta1.VMPodScrape{}
 
-	//list all namespaces for rules with selector
+	//list all namespaces for pods with selector
 	if namespaces == nil {
 		log.Info("listing all namespaces for podScrapes")
-		servMons := &victoriametricsv1beta1.VMPodScrapeList{}
-		err = rclient.List(ctx, servMons, &client.ListOptions{LabelSelector: podScrapeSelector})
+		podScrapes := &victoriametricsv1beta1.VMPodScrapeList{}
+		err = rclient.List(ctx, podScrapes, &client.ListOptions{LabelSelector: podScrapeSelector})
 		if err != nil {
 			return nil, fmt.Errorf("cannot list podScrapes from all namespaces: %w", err)
 		}
-		podScrapesCombined = append(podScrapesCombined, servMons.Items...)
+		podScrapesCombined = append(podScrapesCombined, podScrapes.Items...)
 
 	} else {
 		for _, ns := range namespaces {
 			listOpts := &client.ListOptions{Namespace: ns, LabelSelector: podScrapeSelector}
-			servMons := &victoriametricsv1beta1.VMPodScrapeList{}
-			err = rclient.List(ctx, servMons, listOpts)
+			podScrapes := &victoriametricsv1beta1.VMPodScrapeList{}
+			err = rclient.List(ctx, podScrapes, listOpts)
 			if err != nil {
 				return nil, fmt.Errorf("cannot list podscrapes at namespace: %s, err: %w", ns, err)
 			}
-			podScrapesCombined = append(podScrapesCombined, servMons.Items...)
+			podScrapesCombined = append(podScrapesCombined, podScrapes.Items...)
 
 		}
 	}
 
-	log.Info("filtering namespaces to select PodScrapes from",
-		"namespace", cr.Namespace, "vmagent", cr.Name)
 	for _, podScrape := range podScrapesCombined {
 		pm := podScrape.DeepCopy()
 		res[podScrape.Namespace+"/"+podScrape.Name] = pm

controllers/factory/vmagent.go

@@ -615,7 +615,7 @@ func CreateOrUpdateTlsAssets(ctx context.Context, cr *victoriametricsv1beta1.VMA
 		if err != nil {
 			return fmt.Errorf("cannot create tls asset secret: %s for vmagent: %s, err :%w", tlsAssetsSecret.Name, cr.Name, err)
 		}
-		log.Info("create new tls asset secret: %s, for vmagent: %s", tlsAssetsSecret.Name, cr.Name)
+		log.Info("create new tls asset for vmagent", "secret_name", tlsAssetsSecret.Name, "vmagent", cr.Name)
 		return nil
 	}
 	for annotation, value := range currentAssetSecret.Annotations {

controllers/factory/vmalert.go

@@ -631,13 +631,13 @@ func CreateOrUpdateTlsAssetsForVMAlert(ctx context.Context, cr *victoriametricsv
 	err = rclient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: tlsAssetsSecret.Name}, currentAssetSecret)
 	if err != nil {
 		if !errors.IsNotFound(err) {
-			return fmt.Errorf("cannot get existing tls secret: %s, for vmagent: %s, err: %w", tlsAssetsSecret.Name, cr.Name, err)
+			return fmt.Errorf("cannot get existing tls secret: %s, for vmalert: %s, err: %w", tlsAssetsSecret.Name, cr.Name, err)
 		}
 		err := rclient.Create(ctx, tlsAssetsSecret)
 		if err != nil {
-			return fmt.Errorf("cannot create tls asset secret: %s for vmagent: %s, err :%w", tlsAssetsSecret.Name, cr.Name, err)
+			return fmt.Errorf("cannot create tls asset secret: %s for vmalert: %s, err :%w", tlsAssetsSecret.Name, cr.Name, err)
 		}
-		log.Info("create new tls asset secret: %s, for vmagent: %s", tlsAssetsSecret.Name, cr.Name)
+		log.Info("create new tls asset secret for vmalert", "secret_name", tlsAssetsSecret.Name, "vmalert", cr.Name)
 		return nil
 	}
 	for annotation, value := range currentAssetSecret.Annotations {
@@ -651,8 +651,11 @@ func loadTLSAssetsForVMAlert(ctx context.Context, rclient client.Client, cr *vic
 	nsSecretCache := make(map[string]*corev1.Secret)
 	nsConfigMapCache := make(map[string]*corev1.ConfigMap)
 	tlsConfigs := []*victoriametricsv1beta1.TLSConfig{}
-	if cr.Spec.Notifier.TLSConfig != nil {
-		tlsConfigs = append(tlsConfigs, cr.Spec.Notifier.TLSConfig)
+
+	for _, notifier := range cr.Spec.Notifiers {
+		if notifier.TLSConfig != nil {
+			tlsConfigs = append(tlsConfigs, notifier.TLSConfig)
+		}
 	}
 	if cr.Spec.RemoteRead != nil && cr.Spec.RemoteRead.TLSConfig != nil {
 		tlsConfigs = append(tlsConfigs, cr.Spec.RemoteRead.TLSConfig)

controllers/factory/vmalert_test.go

@@ -222,6 +222,71 @@ func TestCreateOrUpdateVMAlert(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "with-notifiers-tls",
+			args: args{
+				cr: &victoriametricsv1beta1.VMAlert{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic-vmalert",
+						Namespace: "default",
+					},
+					Spec: victoriametricsv1beta1.VMAlertSpec{
+						Notifiers: []victoriametricsv1beta1.VMAlertNotifierSpec{
+							{
+								URL: "http://another-alertmanager",
+								TLSConfig: &victoriametricsv1beta1.TLSConfig{
+									CAFile:   "/tmp/ca",
+									CertFile: "/tmp/cert",
+									KeyFile:  "/tmp/key",
+								},
+							},
+						},
+						Datasource: victoriametricsv1beta1.VMAlertDatasourceSpec{
+							URL: "http://some-vm-datasource",
+							TLSConfig: &victoriametricsv1beta1.TLSConfig{
+								CA: victoriametricsv1beta1.SecretOrConfigMap{
+									Secret: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
+								},
+								Cert: victoriametricsv1beta1.SecretOrConfigMap{
+									Secret: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
+								},
+								KeySecret: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "key"},
+							},
+						},
+						RemoteWrite: &victoriametricsv1beta1.VMAlertRemoteWriteSpec{
+							URL: "http://vm-insert-url",
+							TLSConfig: &victoriametricsv1beta1.TLSConfig{
+								CA: victoriametricsv1beta1.SecretOrConfigMap{
+									ConfigMap: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
+								},
+								Cert: victoriametricsv1beta1.SecretOrConfigMap{
+									ConfigMap: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "ca"},
+								},
+								KeySecret: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "datasource-tls"}, Key: "key"},
+							},
+						},
+					},
+				},
+				c: config.MustGetBaseConfig(),
+			},
+			predefinedObjects: []runtime.Object{
+				&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "datasource-tls",
+						Namespace: "default",
+					},
+					Data: map[string][]byte{"ca": []byte(`sa`), "cert": []byte(`cert-data`), "key": []byte(`"key-data"`)},
+				},
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "datasource-tls",
+						Namespace: "default",
+					},
+					Data: map[string]string{"ca": "ca-data", "cert": "cert-data"},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

docs/api.MD

@@ -1038,7 +1038,7 @@ VMClusterStatus defines the observed state of VMCluster
 
 ## VMNodeScrape
 
-VMNodeScrape is the Schema for the vmnodescrapes API
+VMNodeScrape defines discovery for targets placed on kubernetes nodes, usually its node-exporters and other host services. InternalIP is used as __address__ for scraping.
 
 | Field | Description | Scheme | Required |
 | ----- | ----------- | ------ | -------- |
@@ -1061,7 +1061,7 @@ VMNodeScrapeList contains a list of VMNodeScrape
 
 ## VMNodeScrapeSpec
 
-VMNodeScrapeSpec defines discovery for targets placed on kubernetes nodes, usually its node-exporters and other host services. InternalIP is used as __address__ for scraping.
+VMNodeScrapeSpec defines specification for VMNodeScrape.
 
 | Field | Description | Scheme | Required |
 | ----- | ----------- | ------ | -------- |

e2e/vmalert_test.go

@@ -58,7 +58,7 @@ var _ = Describe("test  vmalert Controller", func() {
 					}, 60, 1).Should(BeEmpty())
 
 				})
-				It("should create with remote read tls", func() {
+				It("should create with remote read and notifier tls", func() {
 					tlsSecretName := "vmalert-remote-tls"
 					tlsSecret := &corev1.Secret{
 						ObjectMeta: metav1.ObjectMeta{
@@ -80,8 +80,16 @@ var _ = Describe("test  vmalert Controller", func() {
 						},
 						Spec: operator.VMAlertSpec{
 							ReplicaCount: pointer.Int32Ptr(1),
-							Notifier:     &operator.VMAlertNotifierSpec{URL: "http://alert-manager-url:9093"},
-							Secrets:      []string{tlsSecretName},
+							Notifiers: []operator.VMAlertNotifierSpec{
+								{
+									URL: "http://alert-manager-url:9093",
+									TLSConfig: &operator.TLSConfig{
+										CertFile: path.Join(factory.SecretsDir, tlsSecretName, "remote-cert"),
+										KeyFile:  path.Join(factory.SecretsDir, tlsSecretName, "remote-key"),
+										CAFile:   path.Join(factory.SecretsDir, tlsSecretName, "remote-ca"),
+									},
+								}},
+							Secrets: []string{tlsSecretName},
 							Datasource: operator.VMAlertDatasourceSpec{
 								URL: "http://some-datasource-url:8428",
 								TLSConfig: &operator.TLSConfig{