Merge pull request #317 from bharath-b-rh/cm-639

CM-639: Adds metrics service creation for istio-csr

Author: openshift-merge-bot[bot]

bindata/istio-csr/cert-manager-istio-csr-metrics-service.yaml

@@ -0,0 +1,20 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: cert-manager-istio-csr-metrics
+  namespace: cert-manager
+  labels:
+    app: cert-manager-istio-csr-metrics
+    app.kubernetes.io/name: cert-manager-istio-csr
+    app.kubernetes.io/instance: cert-manager-istio-csr
+    app.kubernetes.io/version: v0.14.2
+    app.kubernetes.io/managed-by: cert-manager-operator
+spec:
+  type: ClusterIP
+  ports:
+    - port: 9402
+      targetPort: 9402
+      protocol: TCP
+      name: metrics
+  selector:
+    app: cert-manager-istio-csr

hack/update-istio-csr-manifests.sh

@@ -40,10 +40,7 @@ mkdir -p bindata/istio-csr
   kind=$(echo "$item" | ./bin/yq eval '.kind' - | tr '[:upper:]' '[:lower:]')
 
   # skip unused manifests
-  if [[ "${name}-${kind}" == "cert-manager-istio-csr-metrics-service" || \
-        "${name}-${kind}" == "cert-manager-istio-csr-dynamic-istiod-rolebinding" \
-  ]]; then
-    
+  if [[ "${name}-${kind}" == "cert-manager-istio-csr-dynamic-istiod-rolebinding" ]]; then
     continue
   fi
 

pkg/controller/istiocsr/certificates.go

@@ -5,7 +5,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
@@ -23,11 +23,7 @@ func (r *Reconciler) createOrApplyCertificates(istiocsr *v1alpha1.IstioCSR, reso
 	certificateName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling certificate resource", "name", certificateName)
 	fetched := &certmanagerv1.Certificate{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s certificate resource already exists", certificateName)
 	}

pkg/controller/istiocsr/client.go

@@ -6,7 +6,6 @@ import (
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -74,7 +73,7 @@ func (c *ctrlClientImpl) Update(
 func (c *ctrlClientImpl) UpdateWithRetry(
 	ctx context.Context, obj client.Object, opts ...client.UpdateOption,
 ) error {
-	key := types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}
+	key := client.ObjectKeyFromObject(obj)
 	if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		current := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(client.Object)
 		if err := c.Client.Get(ctx, key, current); err != nil {

pkg/controller/istiocsr/constants.go

@@ -111,6 +111,7 @@ const (
 	roleBindingAssetName        = "istio-csr/cert-manager-istio-csr-rolebinding.yaml"
 	roleBindingLeasesAssetName  = "istio-csr/cert-manager-istio-csr-leases-rolebinding.yaml"
 	serviceAssetName            = "istio-csr/cert-manager-istio-csr-service.yaml"
+	metricsServiceAssetName     = "istio-csr/cert-manager-istio-csr-metrics-service.yaml"
 	serviceAccountAssetName     = "istio-csr/cert-manager-istio-csr-serviceaccount.yaml"
 )
 

pkg/controller/istiocsr/deployments.go

@@ -11,11 +11,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/apis/core"
 	corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
@@ -39,11 +37,7 @@ func (r *Reconciler) createOrApplyDeployments(istiocsr *v1alpha1.IstioCSR, resou
 	deploymentName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling deployment resource", "name", deploymentName)
 	fetched := &appsv1.Deployment{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s deployment resource already exists", deploymentName)
 	}
@@ -336,7 +330,7 @@ func updateVolumeWithIssuerCA(deployment *appsv1.Deployment) {
 
 func (r *Reconciler) getIssuer(istiocsr *v1alpha1.IstioCSR) (client.Object, error) {
 	issuerRefKind := strings.ToLower(istiocsr.Spec.IstioCSRConfig.CertManager.IssuerRef.Kind)
-	namespacedName := types.NamespacedName{
+	key := client.ObjectKey{
 		Name:      istiocsr.Spec.IstioCSRConfig.CertManager.IssuerRef.Name,
 		Namespace: istiocsr.Spec.IstioCSRConfig.Istio.Namespace,
 	}
@@ -349,8 +343,8 @@ func (r *Reconciler) getIssuer(istiocsr *v1alpha1.IstioCSR) (client.Object, erro
 		object = &certmanagerv1.Issuer{}
 	}
 
-	if err := r.Get(r.ctx, namespacedName, object); err != nil {
-		return nil, fmt.Errorf("failed to fetch %q issuer: %w", namespacedName, err)
+	if err := r.Get(r.ctx, key, object); err != nil {
+		return nil, fmt.Errorf("failed to fetch %q issuer: %w", key, err)
 	}
 	return object, nil
 }
@@ -360,7 +354,7 @@ func (r *Reconciler) createCAConfigMap(istiocsr *v1alpha1.IstioCSR, issuerConfig
 		return nil
 	}
 
-	secretKey := types.NamespacedName{
+	secretKey := client.ObjectKey{
 		Name:      issuerConfig.CA.SecretName,
 		Namespace: istiocsr.Spec.IstioCSRConfig.Istio.Namespace,
 	}
@@ -372,7 +366,7 @@ func (r *Reconciler) createCAConfigMap(istiocsr *v1alpha1.IstioCSR, issuerConfig
 		return err
 	}
 
-	configmapKey := types.NamespacedName{
+	configmapKey := client.ObjectKey{
 		Name:      istiocsrCAConfigMapName,
 		Namespace: istiocsr.GetNamespace(),
 	}

pkg/controller/istiocsr/rbacs.go

@@ -5,8 +5,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/types"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
@@ -61,14 +59,14 @@ func (r *Reconciler) createOrApplyClusterRoles(istiocsr *v1alpha1.IstioCSR, reso
 		exist    bool
 		err      error
 		roleName string
-		key      types.NamespacedName
+		key      client.ObjectKey
 		fetched  = &rbacv1.ClusterRole{}
 	)
 	r.log.V(4).Info("reconciling clusterrole resource created for istiocsr", "namespace", istiocsr.GetNamespace(), "name", istiocsr.GetName())
 	if istiocsr.Status.ClusterRole != "" {
 		roleName = fmt.Sprintf("%s/%s", desired.GetNamespace(), istiocsr.Status.ClusterRole)
 		fetched = &rbacv1.ClusterRole{}
-		key = types.NamespacedName{
+		key = client.ObjectKey{
 			Name:      istiocsr.Status.ClusterRole,
 			Namespace: desired.GetNamespace(),
 		}
@@ -154,14 +152,14 @@ func (r *Reconciler) createOrApplyClusterRoleBindings(istiocsr *v1alpha1.IstioCS
 		exist           bool
 		err             error
 		roleBindingName string
-		key             types.NamespacedName
+		key             client.ObjectKey
 		fetched         = &rbacv1.ClusterRoleBinding{}
 	)
 	r.log.V(4).Info("reconciling clusterrolebinding resource created for istiocsr", "namespace", istiocsr.GetNamespace(), "name", istiocsr.GetName())
 	if istiocsr.Status.ClusterRoleBinding != "" {
 		roleBindingName = fmt.Sprintf("%s/%s", desired.GetNamespace(), istiocsr.Status.ClusterRoleBinding)
 		fetched = &rbacv1.ClusterRoleBinding{}
-		key = types.NamespacedName{
+		key = client.ObjectKey{
 			Name:      istiocsr.Status.ClusterRoleBinding,
 			Namespace: desired.GetNamespace(),
 		}
@@ -243,11 +241,7 @@ func (r *Reconciler) createOrApplyRoles(istiocsr *v1alpha1.IstioCSR, resourceLab
 	roleName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling role resource", "name", roleName)
 	fetched := &rbacv1.Role{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s role resource already exists", roleName)
 	}
@@ -287,11 +281,7 @@ func (r *Reconciler) createOrApplyRoleBindings(istiocsr *v1alpha1.IstioCSR, serv
 	roleBindingName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling rolebinding resource", "name", roleBindingName)
 	fetched := &rbacv1.RoleBinding{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s rolebinding resource already exists", roleBindingName)
 	}
@@ -333,11 +323,7 @@ func (r *Reconciler) createOrApplyRoleForLeases(istiocsr *v1alpha1.IstioCSR, res
 	roleName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling role for lease resource", "name", roleName)
 	fetched := &rbacv1.Role{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s role resource already exists", roleName)
 	}
@@ -377,11 +363,7 @@ func (r *Reconciler) createOrApplyRoleBindingForLeases(istiocsr *v1alpha1.IstioC
 	roleBindingName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling rolebinding for lease resource", "name", roleBindingName)
 	fetched := &rbacv1.RoleBinding{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s rolebinding resource already exists", roleBindingName)
 	}

pkg/controller/istiocsr/serviceaccounts.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
 	"github.com/openshift/cert-manager-operator/pkg/operator/assets"
@@ -16,11 +16,7 @@ func (r *Reconciler) createOrApplyServiceAccounts(istiocsr *v1alpha1.IstioCSR, r
 	serviceAccountName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
 	r.log.V(4).Info("reconciling serviceaccount resource", "name", serviceAccountName)
 	fetched := &corev1.ServiceAccount{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(desired), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s serviceaccount resource already exists", serviceAccountName)
 	}

pkg/controller/istiocsr/services.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openshift/cert-manager-operator/api/operator/v1alpha1"
 	"github.com/openshift/cert-manager-operator/pkg/operator/assets"
@@ -16,42 +16,48 @@ const (
 )
 
 func (r *Reconciler) createOrApplyServices(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string, istioCSRCreateRecon bool) error {
-	desired := r.getServiceObject(istiocsr, resourceLabels)
+	service := r.getServiceObject(istiocsr, resourceLabels)
+	if err := r.createOrApplyService(istiocsr, service, istioCSRCreateRecon); err != nil {
+		return err
+	}
+	if err := r.updateGRPCEndpointInStatus(istiocsr, service); err != nil {
+		return FromClientError(err, "failed to update %s/%s istiocsr status with %s service endpoint info", istiocsr.GetNamespace(), istiocsr.GetName(), service.GetName())
+	}
 
-	serviceName := fmt.Sprintf("%s/%s", desired.GetNamespace(), desired.GetName())
+	metricsService := r.getMetricsServiceObject(istiocsr, resourceLabels)
+	if err := r.createOrApplyService(istiocsr, metricsService, istioCSRCreateRecon); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (r *Reconciler) createOrApplyService(istiocsr *v1alpha1.IstioCSR, svc *corev1.Service, istioCSRCreateRecon bool) error {
+	serviceName := fmt.Sprintf("%s/%s", svc.GetNamespace(), svc.GetName())
 	r.log.V(4).Info("reconciling service resource", "name", serviceName)
 	fetched := &corev1.Service{}
-	key := types.NamespacedName{
-		Name:      desired.GetName(),
-		Namespace: desired.GetNamespace(),
-	}
-	exist, err := r.Exists(r.ctx, key, fetched)
+	exist, err := r.Exists(r.ctx, client.ObjectKeyFromObject(svc), fetched)
 	if err != nil {
 		return FromClientError(err, "failed to check %s service resource already exists", serviceName)
 	}
 
 	if exist && istioCSRCreateRecon {
 		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeWarning, "ResourceAlreadyExists", "%s service resource already exists, maybe from previous installation", serviceName)
 	}
-	if exist && hasObjectChanged(desired, fetched) {
+	if exist && hasObjectChanged(svc, fetched) {
 		r.log.V(1).Info("service has been modified, updating to desired state", "name", serviceName)
-		if err := r.UpdateWithRetry(r.ctx, desired); err != nil {
+		if err := r.UpdateWithRetry(r.ctx, svc); err != nil {
 			return FromClientError(err, "failed to update %s service resource", serviceName)
 		}
 		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "service resource %s reconciled back to desired state", serviceName)
 	} else {
 		r.log.V(4).Info("service resource already exists and is in expected state", "name", serviceName)
 	}
 	if !exist {
-		if err := r.Create(r.ctx, desired); err != nil {
+		if err := r.Create(r.ctx, svc); err != nil {
 			return FromClientError(err, "failed to create %s service resource", serviceName)
 		}
 		r.eventRecorder.Eventf(istiocsr, corev1.EventTypeNormal, "Reconciled", "service resource %s created", serviceName)
 	}
-
-	if err := r.updateGRPCEndpointInStatus(istiocsr, desired); err != nil {
-		return FromClientError(err, "failed to update %s/%s istiocsr status with %s service endpoint info", istiocsr.GetNamespace(), istiocsr.GetName(), serviceName)
-	}
 	return nil
 }
 
@@ -65,6 +71,13 @@ func (r *Reconciler) getServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabel
 	return service
 }
 
+func (r *Reconciler) getMetricsServiceObject(istiocsr *v1alpha1.IstioCSR, resourceLabels map[string]string) *corev1.Service {
+	service := decodeServiceObjBytes(assets.MustAsset(metricsServiceAssetName))
+	updateNamespace(service, istiocsr.GetNamespace())
+	updateResourceLabels(service, resourceLabels)
+	return service
+}
+
 func updateServicePort(service *corev1.Service, port int32) {
 	for i, servicePort := range service.Spec.Ports {
 		if servicePort.Name == grpcServicePortName && port != 0 {

pkg/controller/istiocsr/utils.go

@@ -11,10 +11,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/util/retry"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -45,7 +43,7 @@ func init() {
 
 // updateStatus is for updating the status subresource of istiocsr.openshift.operator.io.
 func (r *Reconciler) updateStatus(ctx context.Context, changed *v1alpha1.IstioCSR) error {
-	namespacedName := types.NamespacedName{Name: changed.Name, Namespace: changed.Namespace}
+	namespacedName := client.ObjectKeyFromObject(changed)
 	if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		r.log.V(4).Info("updating istiocsr.openshift.operator.io status", "request", namespacedName)
 		current := &v1alpha1.IstioCSR{}
@@ -68,7 +66,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, changed *v1alpha1.IstioCS
 
 // addFinalizer adds finalizer to istiocsr.openshift.operator.io resource.
 func (r *Reconciler) addFinalizer(ctx context.Context, istiocsr *v1alpha1.IstioCSR) error {
-	namespacedName := types.NamespacedName{Name: istiocsr.Name, Namespace: istiocsr.Namespace}
+	namespacedName := client.ObjectKeyFromObject(istiocsr)
 	if !controllerutil.ContainsFinalizer(istiocsr, finalizer) {
 		if !controllerutil.AddFinalizer(istiocsr, finalizer) {
 			return fmt.Errorf("failed to create %q istiocsr.openshift.operator.io object with finalizers added", namespacedName)
@@ -91,7 +89,7 @@ func (r *Reconciler) addFinalizer(ctx context.Context, istiocsr *v1alpha1.IstioC
 
 // removeFinalizer removes finalizers added to istiocsr.openshift.operator.io resource.
 func (r *Reconciler) removeFinalizer(ctx context.Context, istiocsr *v1alpha1.IstioCSR, finalizer string) error {
-	namespacedName := types.NamespacedName{Name: istiocsr.Name, Namespace: istiocsr.Namespace}
+	namespacedName := client.ObjectKeyFromObject(istiocsr)
 	if controllerutil.ContainsFinalizer(istiocsr, finalizer) {
 		if !controllerutil.RemoveFinalizer(istiocsr, finalizer) {
 			return fmt.Errorf("failed to create %q istiocsr.openshift.operator.io object with finalizers removed", namespacedName)