Fix registry viewer host alias resolution (devfile#51)

* Fix registry viewer host alias resolution

Signed-off-by: Kim Tsao <ktsao@redhat.com>

* Add a fake kubeconfig to fix CI tests

Signed-off-by: Kim Tsao <ktsao@redhat.com>

* fix path

Signed-off-by: Kim Tsao <ktsao@redhat.com>

* Undo controller test changes

Signed-off-by: Kim Tsao <ktsao@redhat.com>

* update integration tests

Signed-off-by: Kim Tsao <ktsao@redhat.com>

* Address review comments

Signed-off-by: Kim Tsao <ktsao@redhat.com>

---------

Signed-off-by: Kim Tsao <ktsao@redhat.com>
Signed-off-by: thepetk <thepetk@gmail.com>

.github/workflows/ci.yaml

@@ -67,7 +67,7 @@ jobs:
     -
       name: Check CRD manifest generation
       run: make manifests
-    - 
+    -
       name: Run unit tests
       run: make test
     - 

controllers/devfileregistry_controller.go

@@ -183,13 +183,6 @@ func (r *DevfileRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 			log.Error(err, "Failed to update DevfileRegistry status")
 			return ctrl.Result{Requeue: true}, err
 		}
-
-		//update the config map
-		result, err = r.ensure(ctx, devfileRegistry, &corev1.ConfigMap{}, labels, "")
-		if result != nil {
-			return *result, err
-		}
-
 	}
 
 	// Update condition status
@@ -224,8 +217,7 @@ func (r *DevfileRegistryReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&appsv1.Deployment{}).
 		Owns(&corev1.Service{}).
 		Owns(&corev1.PersistentVolumeClaim{}).
-		Owns(&networkingv1.Ingress{}).
-		Owns(&corev1.ConfigMap{})
+		Owns(&networkingv1.Ingress{})
 
 	// If on OpenShift, mark routes as owned by the controller
 	if config.ControllerCfg.IsOpenShift() {

controllers/ensure.go

@@ -66,9 +66,6 @@ func (r *DevfileRegistryReconciler) ensure(ctx context.Context, cr *registryv1al
 	case *networkingv1.Ingress:
 		ingress, _ := resource.(*networkingv1.Ingress)
 		err = r.updateIngress(ctx, cr, ingressDomain, ingress)
-	case *corev1.ConfigMap:
-		configMap, _ := resource.(*corev1.ConfigMap)
-		err = r.updateConfigMap(ctx, cr, configMap)
 	}
 	if err != nil {
 		r.Log.Error(err, "Failed to update "+resourceType)

controllers/suite_test.go

@@ -17,6 +17,7 @@
 package controllers
 
 import (
+	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme"
 	"path/filepath"
 	"testing"
 
@@ -29,7 +30,6 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -126,7 +126,7 @@ func getClusterDevfileRegistriesListCR(name string, namespace string, registryNa
 	return &ClusterDevfileRegistriesList{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: ApiVersion,
-			Kind:       "ClusterDevfileRegistriesList",
+			Kind:       string(ClusterListType),
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -149,7 +149,7 @@ func getDevfileRegistriesListCR(name string, namespace string, registryName stri
 	return &DevfileRegistriesList{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: ApiVersion,
-			Kind:       "DevfileRegistriesList",
+			Kind:       string(NamespaceListType),
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -172,6 +172,7 @@ func deleteCRList(drlLookupKey types.NamespacedName, f ListType) {
 
 	cl := &ClusterDevfileRegistriesList{}
 	nl := &DevfileRegistriesList{}
+
 	// Delete
 	Eventually(func() error {
 		if f == ClusterListType {

controllers/update.go

@@ -88,6 +88,23 @@ func (r *DevfileRegistryReconciler) updateDeployment(ctx context.Context, cr *re
 	if len(dep.Spec.Template.Spec.Containers) > 2 {
 		viewerImage := registry.GetRegistryViewerImage(cr)
 		viewerImageContainer := dep.Spec.Template.Spec.Containers[2]
+
+		//determine if the NEXT_PUBLIC_ANALYTICS_WRITE_KEY env needs updating
+		viewerKey := cr.Spec.Telemetry.RegistryViewerWriteKey
+		if viewerImageContainer.Env[0].Value != viewerKey {
+			r.Log.Info("Updating NEXT_PUBLIC_ANALYTICS_WRITE_KEY ", "value", viewerKey)
+			viewerImageContainer.Env[0].Value = viewerKey
+			needsUpdating = true
+		}
+
+		//determine if the DEVFILE_REGISTRIES env needs updating.  This will only occur on initial deployment since object name is unique
+		newDRValue := fmt.Sprintf(`[{"name": "%s","url": "http://localhost:8080","fqdn": "%s"}]`, cr.ObjectMeta.Name, cr.Status.URL)
+		if viewerImageContainer.Env[1].Value != newDRValue {
+			r.Log.Info("Updating DEVFILE_REGISTRIES ", "value", newDRValue)
+			viewerImageContainer.Env[1].Value = newDRValue
+			needsUpdating = true
+		}
+
 		if viewerImageContainer.Image != viewerImage {
 			viewerImageContainer.Image = viewerImage
 			needsUpdating = true
@@ -202,16 +219,3 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr
 	}
 	return nil
 }
-
-func (r *DevfileRegistryReconciler) updateConfigMap(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, configMap *corev1.ConfigMap) error {
-
-	viewerEnvfile := fmt.Sprintf(`
-NEXT_PUBLIC_ANALYTICS_WRITE_KEY=%s
-DEVFILE_REGISTRIES=[{"name":"%s","url":"http://localhost:8080","fqdn":"%s"}]`,
-		cr.Spec.Telemetry.RegistryViewerWriteKey, cr.ObjectMeta.Name, cr.Status.URL)
-
-	configMap.Data[".env.registry-viewer"] = viewerEnvfile
-
-	return r.Update(ctx, configMap)
-
-}

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/stretchr/testify v1.8.1
 	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.26.2
+	k8s.io/apiextensions-apiserver v0.26.1
 	k8s.io/apimachinery v0.26.2
 	k8s.io/client-go v0.26.2
 	sigs.k8s.io/controller-runtime v0.14.5
@@ -87,7 +88,6 @@ require (
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/apiextensions-apiserver v0.26.1 // indirect
 	k8s.io/component-base v0.26.1 // indirect
 	k8s.io/klog/v2 v2.80.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 // indirect

pkg/registry/deployment.go

@@ -280,30 +280,6 @@ func GenerateDeployment(cr *registryv1alpha1.DevfileRegistry, scheme *runtime.Sc
 					]`, cr.ObjectMeta.Name, cr.Status.URL),
 				},
 			},
-			VolumeMounts: []corev1.VolumeMount{
-				{
-					Name:      "viewer-env-file",
-					MountPath: "/app/.env.production",
-					SubPath:   ".env.production",
-					ReadOnly:  true,
-				},
-			},
-		})
-		dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, corev1.Volume{
-			Name: "viewer-env-file",
-			VolumeSource: corev1.VolumeSource{
-				ConfigMap: &corev1.ConfigMapVolumeSource{
-					LocalObjectReference: corev1.LocalObjectReference{
-						Name: ConfigMapName(cr.Name),
-					},
-					Items: []corev1.KeyToPath{
-						{
-							Key:  ".env.registry-viewer",
-							Path: ".env.production",
-						},
-					},
-				},
-			},
 		})
 	} else {
 		// Set environment variable to run index server in headless mode

pkg/test/test_utils.go

@@ -31,11 +31,12 @@ import (
 type ListType string
 
 const (
-	ClusterListType   ListType = "ClusterDevfileRegistriesList"
-	NamespaceListType ListType = "DevfileRegistriesList"
-	ApiVersion                 = "registry.devfile.io/v1alpha1"
-	Timeout                    = time.Second * 10
-	Interval                   = time.Millisecond * 250
+	ClusterListType     ListType = "ClusterDevfileRegistriesList"
+	NamespaceListType   ListType = "DevfileRegistriesList"
+	DevfileRegistryType ListType = "DevfileRegistry"
+	ApiVersion                   = "registry.devfile.io/v1alpha1"
+	Timeout                      = time.Second * 10
+	Interval                     = time.Millisecond * 250
 )
 
 // GetNewUnstartedTestServer is a mock test index server that supports just the v2 index schema

tests/integration/examples/update/devfileregistry-new.yaml

@@ -8,4 +8,7 @@ spec:
   storage:
     enabled: false
   tls:
-    enabled: true
+    enabled: true
+  telemetry:
+    key: registry-key
+    registryViewerWriteKey: viewer-key

tests/integration/pkg/client/pod.go

@@ -48,6 +48,31 @@ func (w *K8sClient) WaitForPodRunningByLabelWithNamespace(label string, namespac
 	}
 }
 
+// WaitForPodFailedByLabelWithNamespace waits for the pod matching the specified label in a specified namespace to become running
+// An error is returned if the pod does not exist or the timeout is reached
+func (w *K8sClient) WaitForPodFailedByLabelWithNamespace(label string, namespace string) (deployed bool, err error) {
+	timeout := time.After(6 * time.Minute)
+	tick := time.Tick(1 * time.Second)
+
+	for {
+		select {
+		case <-timeout:
+			return false, errors.New("timed out")
+		case <-tick:
+			err := w.WaitForFailedPodBySelector(namespace, label, 3*time.Minute)
+			if err == nil {
+				return true, nil
+			}
+		}
+	}
+}
+
+// WaitForPodFailedByLabel waits for the pod matching the specified label to become running
+// An error is returned if the pod does not exist or the timeout is reached
+func (w *K8sClient) WaitForPodFailedByLabel(label string) (deployed bool, err error) {
+	return w.WaitForPodFailedByLabelWithNamespace(label, config.Namespace)
+}
+
 // WaitForPodRunningByLabel waits for the pod matching the specified label to become running
 // An error is returned if the pod does not exist or the timeout is reached
 func (w *K8sClient) WaitForPodRunningByLabel(label string) (deployed bool, err error) {
@@ -77,6 +102,29 @@ func (w *K8sClient) WaitForRunningPodBySelector(namespace, selector string, time
 	return nil
 }
 
+// WaitForFailedPodBySelector waits up to timeout seconds for all pods in 'namespace' with given 'selector' to enter running state.
+// Returns an error if no pods are found or not all discovered pods enter running state.
+func (w *K8sClient) WaitForFailedPodBySelector(namespace, selector string, timeout time.Duration) error {
+	podList, err := w.ListPods(namespace, selector)
+	if err != nil {
+		return err
+	}
+	if len(podList.Items) == 0 {
+		fmt.Println("No pods for " + selector + " in namespace " + namespace)
+
+		return nil
+	}
+
+	for _, pod := range podList.Items {
+		fmt.Println("Pod " + pod.Name + " created in namespace " + namespace + "...Checking for failure.")
+		if err := w.waitForPodFailing(namespace, pod.Name, timeout); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // ListPods returns the list of currently scheduled or running pods in `namespace` with the given selector
 func (w *K8sClient) ListPods(namespace, selector string) (*v1.PodList, error) {
 	listOptions := metav1.ListOptions{LabelSelector: selector}
@@ -94,6 +142,12 @@ func (w *K8sClient) waitForPodRunning(namespace, podName string, timeout time.Du
 	return wait.PollImmediate(time.Second, timeout, w.isPodRunning(podName, namespace))
 }
 
+// Poll up to timeout seconds for pod to enter running state.
+// Returns an error if the pod never enters the running state.
+func (w *K8sClient) waitForPodFailing(namespace, podName string, timeout time.Duration) error {
+	return wait.PollImmediate(time.Second, timeout, w.isPodFailing(podName, namespace))
+}
+
 // return a condition function that indicates whether the given pod is
 // currently running
 func (w *K8sClient) isPodRunning(podName, namespace string) wait.ConditionFunc {
@@ -111,3 +165,18 @@ func (w *K8sClient) isPodRunning(podName, namespace string) wait.ConditionFunc {
 		return false, nil
 	}
 }
+
+// return a condition function that indicates whether the given pod is
+// currently running
+func (w *K8sClient) isPodFailing(podName, namespace string) wait.ConditionFunc {
+	return func() (bool, error) {
+		pod, _ := w.Kube().CoreV1().Pods(namespace).Get(context.TODO(), podName, metav1.GetOptions{})
+
+		if pod.Status.Phase == v1.PodRunning {
+			return false, nil
+		} else {
+			fmt.Printf("Pod terminated %s\n", podName)
+			return true, nil
+		}
+	}
+}