diff --git a/Makefile b/Makefile index 1c9db5b..b948f4e 100644 --- a/Makefile +++ b/Makefile @@ -103,7 +103,7 @@ generate: controller-gen $(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..." # Build the docker image -docker-build: test +docker-build: docker build . -t ${IMG} # Push the docker image diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index cea1706..b0521d7 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -36,7 +36,7 @@ spec: resources: limits: cpu: 100m - memory: 30Mi + memory: 100Mi requests: cpu: 100m memory: 20Mi diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f266715..fcd40a7 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -21,6 +21,7 @@ rules: - apiGroups: - "" resources: + - configmaps - persistentvolumeclaims - services verbs: diff --git a/controllers/devfileregistry_controller.go b/controllers/devfileregistry_controller.go index f4e63ee..84250f1 100644 --- a/controllers/devfileregistry_controller.go +++ b/controllers/devfileregistry_controller.go @@ -43,7 +43,7 @@ type DevfileRegistryReconciler struct { // +kubebuilder:rbac:groups=registry.devfile.io,resources=devfileregistries,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=registry.devfile.io,resources=devfileregistries/status;devfileregistries/finalizers,verbs=get;update;patch // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=core,resources=services;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=configmaps;services;persistentvolumeclaims,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list; // +kubebuilder:rbac:groups=extensions,resources=ingresses,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=route.openshift.io,resources=routes;routes/custom-host,verbs=get;list;watch;create;update;patch;delete @@ -71,8 +71,10 @@ func (r *DevfileRegistryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er // Generate labels for any subresources generated by the operator labels := registry.LabelsForDevfileRegistry(devfileRegistry.Name) + log.Info("Deploying registry") + // Check if the service already exists, if not create a new one - result, err := r.ensureService(ctx, devfileRegistry, labels) + result, err := r.ensure(ctx, devfileRegistry, &corev1.Service{}, labels, "") if result != nil { return *result, err } @@ -80,13 +82,18 @@ func (r *DevfileRegistryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er // If storage is enabled, create a persistent volume claim if registry.IsStorageEnabled(devfileRegistry) { // Check if the persistentvolumeclaim already exists, if not create a new one - result, err = r.ensurePVC(ctx, devfileRegistry, labels) + result, err = r.ensure(ctx, devfileRegistry, &corev1.PersistentVolumeClaim{}, labels, "") if result != nil { return *result, err } } - result, err = r.ensureDeployment(ctx, devfileRegistry, labels) + result, err = r.ensure(ctx, devfileRegistry, &corev1.ConfigMap{}, labels, "") + if result != nil { + return *result, err + } + + result, err = r.ensure(ctx, devfileRegistry, &appsv1.Deployment{}, labels, "") if result != nil { return *result, err } @@ -102,14 +109,14 @@ func (r *DevfileRegistryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er hostname := devfileRegistry.Spec.K8s.IngressDomain if config.ControllerCfg.IsOpenShift() && hostname == "" { // Check if the route exposing the devfile index exists - result, err = r.ensureRoute(ctx, devfileRegistry, labels) + result, err = r.ensure(ctx, devfileRegistry, &routev1.Route{}, labels, "") if result != nil { return *result, err } // Get the hostname of the generated devfile route devfilesRoute := &routev1.Route{} - err = r.Get(ctx, types.NamespacedName{Name: devfileRegistry.Name + "-devfiles", Namespace: devfileRegistry.Namespace}, devfilesRoute) + err = r.Get(ctx, types.NamespacedName{Name: registry.IngressName(devfileRegistry.Name), Namespace: devfileRegistry.Namespace}, devfilesRoute) if err != nil { // Log an error, but requeue, as the controller's cached kube client likely hasn't registered the new route yet. // See https://github.com/operator-framework/operator-sdk/issues/4013#issuecomment-707267616 for an explanation on why we requeue rather than error out here @@ -120,7 +127,7 @@ func (r *DevfileRegistryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er } else { // Create/update the ingress for the devfile registry hostname = registry.GetDevfileRegistryIngress(devfileRegistry) - result, err = r.ensureIngress(ctx, devfileRegistry, hostname, labels) + result, err = r.ensure(ctx, devfileRegistry, &v1beta1.Ingress{}, labels, hostname) if result != nil { return *result, err } diff --git a/controllers/ensure.go b/controllers/ensure.go index 469bad5..a001b2a 100644 --- a/controllers/ensure.go +++ b/controllers/ensure.go @@ -13,6 +13,7 @@ package controllers import ( "context" + "reflect" registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1" "github.com/devfile/registry-operator/pkg/registry" @@ -22,133 +23,82 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -// ensureService ensures that a service for the devfile registry exists on the cluster and is up to date with the custom resource -func (r *DevfileRegistryReconciler) ensureService(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, labels map[string]string) (*reconcile.Result, error) { - // Check if the service already exists, if not create a new one - svc := &corev1.Service{} - err := r.Get(ctx, types.NamespacedName{Name: registry.ServiceName(cr.Name), Namespace: cr.Namespace}, svc) - if err != nil && errors.IsNotFound(err) { - // Define a new service - svc := registry.GenerateService(cr, r.Scheme, labels) - log.Info("Creating a new Service", "Service.Namespace", svc.Namespace, "Service.Name", svc.Name) - err = r.Create(ctx, svc) - if err != nil { - log.Error(err, "Failed to create new Service", "Service.Namespace", svc.Namespace, "Service.Name", svc.Name) - return &ctrl.Result{}, err - } - return nil, nil - } else if err != nil { - log.Error(err, "Failed to get Service") - return &ctrl.Result{}, err - } - - return nil, nil -} - -// ensureDeployment ensures that a devfile registry deployment exists on the cluster and is up to date with the custom resource -func (r *DevfileRegistryReconciler) ensureDeployment(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, labels map[string]string) (*reconcile.Result, error) { - dep := &appsv1.Deployment{} - err := r.Get(ctx, types.NamespacedName{Name: registry.DeploymentName(cr.Name), Namespace: cr.Namespace}, dep) - if err != nil && errors.IsNotFound(err) { - // Generate a new Deployment template and create it on the cluster - dep = registry.GenerateDeployment(cr, r.Scheme, labels) - - log.Info("Creating a new Deployment", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name) - err = r.Create(ctx, dep) - if err != nil { - log.Error(err, "Failed to create new Deployment", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name) - return &ctrl.Result{}, err - } - return nil, nil - } else if err != nil { - log.Error(err, "Failed to get Deployment") - return &ctrl.Result{}, err - } - - err = r.updateDeployment(ctx, cr, dep) - if err != nil { - log.Error(err, "Failed to update Deployment") - return &ctrl.Result{}, err - } - return nil, nil -} +func (r *DevfileRegistryReconciler) ensure(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, resource runtime.Object, labels map[string]string, ingressDomain string) (*reconcile.Result, error) { + resourceType := reflect.TypeOf(resource).Elem().Name() + resourceName := getResourceName(resource, cr.Name) -func (r *DevfileRegistryReconciler) ensurePVC(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, labels map[string]string) (*reconcile.Result, error) { - // Check if the persistentvolumeclaim already exists, if not create a new one - pvc := &corev1.PersistentVolumeClaim{} - err := r.Get(ctx, types.NamespacedName{Name: registry.PVCName(cr.Name), Namespace: cr.Namespace}, pvc) + // Check to see if the requested resource exists on the cluster. If it doesn't exist, create it and return. + err := r.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: cr.Namespace}, resource) if err != nil && errors.IsNotFound(err) { - // Define a new PVC - pvc := registry.GeneratePVC(cr, r.Scheme, labels) - log.Info("Creating a new PersistentVolumeClaim", "PersistentVolumeClaim.Namespace", pvc.Namespace, "PersistentVolumeClaim.Name", pvc.Name) - err = r.Create(ctx, pvc) + generatedResource := r.generateResourceObject(cr, resource, labels, ingressDomain) + log.Info("Creating a new "+resourceType, resourceType+".Namespace", cr.Namespace+".Name", resourceName) + err = r.Create(ctx, generatedResource) if err != nil { - log.Error(err, "Failed to create new PersistentVolumeClaim", "PersistentVolumeClaim.Namespace", pvc.Namespace, "PersistentVolumeClaim.Name", pvc.Name) + log.Error(err, "Failed to create new "+resourceType, resourceType+".Namespace", cr.Namespace, "Service.Name", cr.Namespace+".Name", resourceName) return &ctrl.Result{}, err } return nil, nil } else if err != nil { - log.Error(err, "Failed to get PersistentVolumeClaim") + log.Error(err, "Failed to get "+resourceType) return &ctrl.Result{}, err } - return nil, nil -} - -func (r *DevfileRegistryReconciler) ensureRoute(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, labels map[string]string) (*reconcile.Result, error) { - route := &routev1.Route{} - err := r.Get(ctx, types.NamespacedName{Name: registry.DevfilesRouteName(cr.Name), Namespace: cr.Namespace}, route) - if err != nil && errors.IsNotFound(err) { - // Define a new route exposing the devfile registry index - route := registry.GenerateRoute(cr, r.Scheme, labels) - log.Info("Creating a new Route", "Route.Namespace", route.Namespace, "Route.Name", route.Name) - err = r.Create(ctx, route) - if err != nil { - log.Error(err, "Failed to create new Route", "Route.Namespace", route.Namespace, "Route.Name", route.Name) - return &ctrl.Result{}, err - } - return nil, nil - } else if err != nil { - log.Error(err, "Failed to get Route") - return &ctrl.Result{}, err + // Update the given resource, if needed + // At this moment, only registry deployments, routes and ingresses need to be updated. + switch resource.(type) { + case *appsv1.Deployment: + dep, _ := resource.(*appsv1.Deployment) + err = r.updateDeployment(ctx, cr, dep) + case *routev1.Route: + route, _ := resource.(*routev1.Route) + err = r.updateRoute(ctx, cr, route) + case *v1beta1.Ingress: + ingress, _ := resource.(*v1beta1.Ingress) + err = r.updateIngress(ctx, cr, ingressDomain, ingress) } - - err = r.updateRoute(ctx, cr, route) if err != nil { - log.Error(err, "Failed to update Route") + log.Error(err, "Failed to update "+resourceType) return &ctrl.Result{}, err } - return nil, nil } -func (r *DevfileRegistryReconciler) ensureIngress(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, hostname string, labels map[string]string) (*reconcile.Result, error) { - ingress := &v1beta1.Ingress{} - err := r.Get(ctx, types.NamespacedName{Name: registry.IngressName(cr.Name), Namespace: cr.Namespace}, ingress) - if err != nil && errors.IsNotFound(err) { - // Define a new ingress exposing the devfile index and oci registry - ingress = registry.GenerateIngress(cr, hostname, r.Scheme, labels) - log.Info("Creating a new Ingress", "Ingress.Namespace", ingress.Namespace, "Ingress.Name", ingress.Name) - err = r.Create(ctx, ingress) - if err != nil { - log.Error(err, "Failed to create new Ingress", "Ingress.Namespace", ingress.Namespace, "Ingress.Name", ingress.Name) - return &ctrl.Result{}, err - } - return nil, nil - } else if err != nil { - log.Error(err, "Failed to get Ingress") - return &ctrl.Result{}, err +func getResourceName(resource runtime.Object, crName string) string { + switch resource.(type) { + case *appsv1.Deployment: + return registry.DeploymentName(crName) + case *corev1.ConfigMap: + return registry.ConfigMapName(crName) + case *corev1.PersistentVolumeClaim: + return registry.PVCName(crName) + case *corev1.Service: + return registry.ServiceName(crName) + case *routev1.Route, *v1beta1.Ingress: + return registry.IngressName(crName) } + return registry.GenericResourceName(crName) +} - err = r.updateIngress(ctx, cr, hostname, ingress) - if err != nil { - log.Error(err, "Failed to update Ingress") - return &ctrl.Result{}, err +func (r *DevfileRegistryReconciler) generateResourceObject(cr *registryv1alpha1.DevfileRegistry, resource runtime.Object, labels map[string]string, ingressDomain string) runtime.Object { + switch resource.(type) { + case *appsv1.Deployment: + return registry.GenerateDeployment(cr, r.Scheme, labels) + case *corev1.ConfigMap: + return registry.GenerateOCIRegistryConfigMap(cr, r.Scheme, labels) + case *corev1.PersistentVolumeClaim: + return registry.GeneratePVC(cr, r.Scheme, labels) + case *corev1.Service: + return registry.GenerateService(cr, r.Scheme, labels) + case *routev1.Route: + return registry.GenerateRoute(cr, r.Scheme, labels) + case *v1beta1.Ingress: + return registry.GenerateIngress(cr, ingressDomain, r.Scheme, labels) } - return nil, nil + return nil } diff --git a/pkg/registry/configmap.go b/pkg/registry/configmap.go new file mode 100644 index 0000000..b3497ff --- /dev/null +++ b/pkg/registry/configmap.go @@ -0,0 +1,56 @@ +// +// Copyright (c) 2020 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation + +package registry + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + + registryv1alpha1 "github.com/devfile/registry-operator/api/v1alpha1" +) + +// GenerateOCIRegistryConfigMap returns a configmap that is used to configure the OCI registry container +func GenerateOCIRegistryConfigMap(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Scheme, labels map[string]string) *corev1.ConfigMap { + configMapData := make(map[string]string, 0) + + registryConfig := ` +version: 0.1 +log: + fields: + service: registry +storage: + cache: + blobdescriptor: inmemory + filesystem: + rootdirectory: /var/lib/registry +http: + addr: :5000 + headers: + X-Content-Type-Options: [nosniff] + debug: + addr: :5001 + prometheus: + enabled: true + path: /metrics` + + configMapData["registry-config.yml"] = registryConfig + + cm := &corev1.ConfigMap{ + ObjectMeta: generateObjectMeta(ConfigMapName(cr.Name), cr.Namespace, labels), + Data: configMapData, + } + + // Set DevfileRegistry instance as the owner and controller + ctrl.SetControllerReference(cr, cm, scheme) + return cm +} diff --git a/pkg/registry/defaults.go b/pkg/registry/defaults.go index 00a8d0e..027de0f 100644 --- a/pkg/registry/defaults.go +++ b/pkg/registry/defaults.go @@ -29,8 +29,10 @@ const ( DevfileRegistryTLSEnabled = true // Defaults/constants for devfile registry services - DevfileIndexPortName = "devfile-registry-metadata" - DevfileIndexPort = 8080 + DevfileIndexPortName = "devfile-registry-metadata" + DevfileIndexPort = 8080 + DevfileMetricsPortName = "devfile-registry-metrics" + DevfileMetricsPort = 5001 ) func GetOCIRegistryImage(cr *registryv1alpha1.DevfileRegistry) string { diff --git a/pkg/registry/deployment.go b/pkg/registry/deployment.go index 5b61cc0..2a61289 100644 --- a/pkg/registry/deployment.go +++ b/pkg/registry/deployment.go @@ -90,6 +90,11 @@ func GenerateDeployment(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Sc Name: DevfileRegistryVolumeName, MountPath: "/var/lib/registry", }, + { + Name: "config", + MountPath: "/etc/docker/registry", + ReadOnly: true, + }, }, }, }, @@ -98,6 +103,22 @@ func GenerateDeployment(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Sc Name: DevfileRegistryVolumeName, VolumeSource: GetDevfileRegistryVolumeSource(cr), }, + { + Name: "config", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: ConfigMapName(cr.Name), + }, + Items: []corev1.KeyToPath{ + { + Key: "registry-config.yml", + Path: "config.yml", + }, + }, + }, + }, + }, }, }, }, diff --git a/pkg/registry/naming.go b/pkg/registry/naming.go index 580f264..488a404 100644 --- a/pkg/registry/naming.go +++ b/pkg/registry/naming.go @@ -11,36 +11,37 @@ package registry +// genericResourceName returns just the name of the custom resource, to be used +func GenericResourceName(devfileRegistryName string) string { + return devfileRegistryName +} + // DeploymentName returns the name of the deployment object associated with the DevfileRegistry CR // Just returns the CR name right now, but extracting to a function to avoid relying on that assumption in the future func DeploymentName(devfileRegistryName string) string { - return devfileRegistryName + return GenericResourceName(devfileRegistryName) } // ServiceName returns the name of the service object associated with the DevfileRegistry CR // Just returns the CR name right now, but extracting to a function to avoid relying on that assumption in the future func ServiceName(devfileRegistryName string) string { - return devfileRegistryName + return GenericResourceName(devfileRegistryName) +} + +// ConfigMapName returns the name of the service object associated with the DevfileRegistry CR +// Just returns the CR name right now, but extracting to a function to avoid relying on that assumption in the future +func ConfigMapName(devfileRegistryName string) string { + return devfileRegistryName + "-registry-config" } // PVCName returns the name of the PVC object associated with the DevfileRegistry CR // Just returns the CR name right now, but extracting to a function to avoid relying on that assumption in the future func PVCName(devfileRegistryName string) string { - return devfileRegistryName + return GenericResourceName(devfileRegistryName) } // IngressName returns the name of the Ingress object associated with the DevfileRegistry CR // Just returns the CR name right now, but extracting to a function to avoid relying on that assumption in the future func IngressName(devfileRegistryName string) string { - return devfileRegistryName -} - -// DevfilesRouteName returns the name of the route object associated with the devfile index route -func DevfilesRouteName(devfileRegistryName string) string { - return devfileRegistryName + "-devfiles" -} - -// OCIRouteName returns the name of the route object associated with the OCI registry route -func OCIRouteName(devfileRegistryName string) string { - return devfileRegistryName + "-oci" + return GenericResourceName(devfileRegistryName) } diff --git a/pkg/registry/route.go b/pkg/registry/route.go index 7dd905c..c826c16 100644 --- a/pkg/registry/route.go +++ b/pkg/registry/route.go @@ -25,7 +25,7 @@ func GenerateRoute(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Scheme, weight := int32(100) route := &routev1.Route{ - ObjectMeta: generateObjectMeta(DevfilesRouteName(cr.Name), cr.Namespace, labels), + ObjectMeta: generateObjectMeta(IngressName(cr.Name), cr.Namespace, labels), Spec: routev1.RouteSpec{ To: routev1.RouteTargetReference{ Kind: "Service", diff --git a/pkg/registry/service.go b/pkg/registry/service.go index ab6576b..877d198 100644 --- a/pkg/registry/service.go +++ b/pkg/registry/service.go @@ -29,6 +29,10 @@ func GenerateService(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Schem Name: DevfileIndexPortName, Port: DevfileIndexPort, }, + { + Name: DevfileMetricsPortName, + Port: DevfileMetricsPort, + }, }, Selector: labels, }, diff --git a/tests/integration/pkg/client/oc.go b/tests/integration/pkg/client/oc.go index f236692..33b66fd 100644 --- a/tests/integration/pkg/client/oc.go +++ b/tests/integration/pkg/client/oc.go @@ -41,3 +41,11 @@ func (w *K8sClient) OcDeleteResource(filePath string) (err error) { } return err } + +// CurlEndpointInContainer execs into the given container in the pod and uses curl to hit the specified endpoint +func (w *K8sClient) CurlEndpointInContainer(pod string, container string, endpoint string) (string, error) { + cmd := exec.Command("oc", "exec", pod, "--namespace", config.Namespace, "-c", container, "--", "curl", endpoint) + outBytes, err := cmd.CombinedOutput() + output := string(outBytes) + return output, err +} diff --git a/tests/integration/pkg/tests/devfileregistry_tests.go b/tests/integration/pkg/tests/devfileregistry_tests.go index 42a9e2b..1112677 100644 --- a/tests/integration/pkg/tests/devfileregistry_tests.go +++ b/tests/integration/pkg/tests/devfileregistry_tests.go @@ -18,6 +18,7 @@ import ( "github.com/devfile/registry-operator/pkg/util" "github.com/devfile/registry-operator/tests/integration/pkg/client" + "github.com/devfile/registry-operator/tests/integration/pkg/config" "github.com/onsi/ginkgo" "github.com/onsi/gomega" ) @@ -52,6 +53,15 @@ var _ = ginkgo.Describe("[Create Devfile Registry resource]", func() { gomega.Expect(err).NotTo(gomega.HaveOccurred()) err = util.WaitForServer(registry.Status.URL, 30*time.Second) gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Verify that the metrics endpoint is running + podList, err := K8sClient.ListPods(config.Namespace, label) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + registryPod := podList.Items[0] + metricsURL := "http://localhost:5001/metrics" + output, err := K8sClient.CurlEndpointInContainer(registryPod.Name, "devfile-registry-bootstrap", metricsURL) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(output).To(gomega.ContainSubstring("registry_storage_cache_total")) }) var _ = ginkgo.AfterEach(func() {