Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix registry viewer host alias resolution #51

Merged
merged 6 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
-
name: Check CRD manifest generation
run: make manifests
-
-
name: Run unit tests
run: make test
-
Expand Down
10 changes: 1 addition & 9 deletions controllers/devfileregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,6 @@
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
}

}

return ctrl.Result{}, nil
Expand All @@ -187,8 +180,7 @@
Owns(&appsv1.Deployment{}).
Owns(&corev1.Service{}).
Owns(&corev1.PersistentVolumeClaim{}).
Owns(&networkingv1.Ingress{}).
Owns(&corev1.ConfigMap{})
Owns(&networkingv1.Ingress{})

Check warning on line 183 in controllers/devfileregistry_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/devfileregistry_controller.go#L183

Added line #L183 was not covered by tests

// If on OpenShift, mark routes as owned by the controller
if config.ControllerCfg.IsOpenShift() {
Expand Down
3 changes: 0 additions & 3 deletions controllers/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package controllers

import (
"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme"
"path/filepath"
"testing"

Expand All @@ -28,7 +29,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"
Expand Down Expand Up @@ -125,7 +125,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,
Expand All @@ -148,7 +148,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,
Expand All @@ -171,6 +171,7 @@ func deleteCRList(drlLookupKey types.NamespacedName, f ListType) {

cl := &ClusterDevfileRegistriesList{}
nl := &DevfileRegistriesList{}

// Delete
Eventually(func() error {
if f == ClusterListType {
Expand Down
30 changes: 17 additions & 13 deletions controllers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,23 @@
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
}

Check warning on line 98 in controllers/update.go

View check run for this annotation

Codecov / codecov/patch

controllers/update.go#L91-L98

Added lines #L91 - L98 were not covered by tests

//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
}

Check warning on line 106 in controllers/update.go

View check run for this annotation

Codecov / codecov/patch

controllers/update.go#L101-L106

Added lines #L101 - L106 were not covered by tests

if viewerImageContainer.Image != viewerImage {
viewerImageContainer.Image = viewerImage
needsUpdating = true
Expand Down Expand Up @@ -202,16 +219,3 @@
}
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)

}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions pkg/registry/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions pkg/test/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/examples/update/devfileregistry-new.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ spec:
storage:
enabled: false
tls:
enabled: true
enabled: true
telemetry:
key: registry-key
registryViewerWriteKey: viewer-key
69 changes: 69 additions & 0 deletions tests/integration/pkg/client/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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}
Expand All @@ -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 {
Expand All @@ -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
}
}
}
Loading
Loading