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 3 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
17 changes: 17 additions & 0 deletions .github/mock_kube_config.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/bash
# copied script from the release-service: https://github.com/redhat-appstudio/release-service-bundles/blob/main/.github/scripts/mock_kube_config.sh
printf 'Creating mock kube config for controller tests\n'
mkdir -vp $HOME/.kube || true
cat <<-EOF > $HOME/.kube/config
apiVersion: v1
kind: Config
clusters:
- cluster:
server: _
name: _
contexts:
- context:
cluster: _
name: _
current-context: _
EOF
6 changes: 5 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ jobs:
-
name: Check CRD manifest generation
run: make manifests
-
-
name: Create mock kube config
run: .github/mock_kube_config.sh
shell: bash
-
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 @@ 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
}

}

return ctrl.Result{}, nil
Expand All @@ -187,8 +180,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() {
Expand Down
60 changes: 60 additions & 0 deletions controllers/devfileregistry_controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
Copyright Red Hat

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers

import (
"github.com/devfile/registry-operator/api/v1alpha1"
. "github.com/devfile/registry-operator/pkg/test"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/types"
)

// The following test is only intended to test creation and updates to the DevfileRegistry CR, while ignoring the fact that routes will fail because they will
// not resolve to an actual URL

var _ = Describe("DevfileRegistry controller test", func() {
Context("When Creating and updating a DevfileRegistry CR with valid values", func() {
It("Should create and update a DevfileRegistry CR Name with Status URL not found", func() {
Expect(k8sClient.Create(ctx, getDevfileRegistryCR(devfileRegistryName, devfileRegistriesNamespace,
image))).Should(Succeed())
drlLookupKey := types.NamespacedName{Name: devfileRegistryName, Namespace: devfileRegistriesNamespace}
dr := &v1alpha1.DevfileRegistry{}
Eventually(func() error {
return k8sClient.Get(ctx, drlLookupKey, dr)
}, Timeout, Interval).Should(Succeed())

Expect(dr.Name).Should(Equal(devfileRegistryName))
Expect(dr.Status.URL).Should(Equal("")) // an empty URL is an indicator that a URL was not resolved
kim-tsao marked this conversation as resolved.
Show resolved Hide resolved

// update values
dr.Spec.Telemetry.RegistryName = devfileRegistryName
dr.Spec.Telemetry.Key = "abcdefghijk"
dr.Spec.DevfileIndex.Image = "quay.io/xyz/devfile-index:next"

Expect(k8sClient.Update(ctx, dr)).Should(Succeed())

//try to update the name - it's immutable, so it should fail
dr.Name = devfileRegistryName + "1"
Expect(k8sClient.Update(ctx, dr)).ShouldNot(Succeed())

//delete all crs
deleteCRList(drlLookupKey, DevfileRegistryType)
})
})

})
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
58 changes: 53 additions & 5 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package controllers

import (
routev1 "github.com/openshift/api/route/v1"
"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme"
"path/filepath"
"testing"

Expand All @@ -28,7 +30,8 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/client-go/kubernetes/scheme"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/api/networking/v1"
kim-tsao marked this conversation as resolved.
Show resolved Hide resolved
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
Expand Down Expand Up @@ -57,6 +60,8 @@ const (
devfileStagingRegistryName = "StagingRegistry"
devfileStagingRegistryURL = "https://registry.stage.devfile.io"
localRegistryName = "LocalRegistry"
devfileRegistryName = "devfile-registry-name"
image = "quay.io/devfile/devfile-index:next"
)

func TestAPIs(t *testing.T) {
Expand All @@ -70,7 +75,8 @@ var _ = BeforeSuite(func() {

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases"),
filepath.Join("..", "hack", "routecrd")}, //known hack to add the "external" openshift Route API to the test environment
ErrorIfCRDPathMissing: true,
}

Expand All @@ -81,6 +87,15 @@ var _ = BeforeSuite(func() {
err = v1alpha1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

err = routev1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

err = v1.AddToScheme(scheme.Scheme)
kim-tsao marked this conversation as resolved.
Show resolved Hide resolved
Expect(err).NotTo(HaveOccurred())

err = appsv1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:scheme

k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
Expand All @@ -105,6 +120,13 @@ var _ = BeforeSuite(func() {
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

err = (&DevfileRegistryReconciler{
Client: k8sManager.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("DevfileRegistryReconciler"),
kim-tsao marked this conversation as resolved.
Show resolved Hide resolved
Scheme: k8sManager.GetScheme(),
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

go func() {
defer GinkgoRecover()
err = k8sManager.Start(ctx)
Expand All @@ -125,7 +147,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 +170,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 @@ -166,26 +188,52 @@ func getDevfileRegistriesListCR(name string, namespace string, registryName stri

}

// getDevfileRegistriesListCR returns a minimally populated DevfileRegistry object for testing
func getDevfileRegistryCR(name string, namespace string, image string) *DevfileRegistry {
return &DevfileRegistry{
TypeMeta: metav1.TypeMeta{
APIVersion: ApiVersion,
Kind: string(DevfileRegistryType),
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: DevfileRegistrySpec{
DevfileIndex: DevfileRegistrySpecContainer{
Image: image,
},
},
}
}

// deleteCRList removes the cluster or namespace CR list from the cluster
func deleteCRList(drlLookupKey types.NamespacedName, f ListType) {

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

// Delete
Eventually(func() error {
if f == ClusterListType {
k8sClient.Get(context.Background(), drlLookupKey, cl)
return k8sClient.Delete(context.Background(), cl)
} else {
} else if f == NamespaceListType {
k8sClient.Get(context.Background(), drlLookupKey, nl)
return k8sClient.Delete(context.Background(), nl)
} else {
k8sClient.Get(context.Background(), drlLookupKey, dl)
return k8sClient.Delete(context.Background(), dl)
}
}, Timeout, Interval).Should(Succeed())

// Wait for delete to finish
Eventually(func() error {
if f == ClusterListType {
return k8sClient.Get(context.Background(), drlLookupKey, cl)
} else if f == NamespaceListType {
return k8sClient.Get(context.Background(), drlLookupKey, nl)
} else {
return k8sClient.Get(context.Background(), drlLookupKey, nl)
}
Expand Down
21 changes: 19 additions & 2 deletions controllers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -203,7 +220,7 @@ func (r *DevfileRegistryReconciler) deleteOldPVCIfNeeded(ctx context.Context, cr
return nil
}

func (r *DevfileRegistryReconciler) updateConfigMap(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, configMap *corev1.ConfigMap) error {
/*func (r *DevfileRegistryReconciler) updateConfigMap(ctx context.Context, cr *registryv1alpha1.DevfileRegistry, configMap *corev1.ConfigMap) error {
kim-tsao marked this conversation as resolved.
Show resolved Hide resolved

viewerEnvfile := fmt.Sprintf(`
NEXT_PUBLIC_ANALYTICS_WRITE_KEY=%s
Expand All @@ -214,4 +231,4 @@ DEVFILE_REGISTRIES=[{"name":"%s","url":"http://localhost:8080","fqdn":"%s"}]`,

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
Loading
Loading