From 80f9aecb4cb84da6f6efa93dae44039c8fc9e533 Mon Sep 17 00:00:00 2001 From: Scott McClements <31139144+smcclem@users.noreply.github.com> Date: Wed, 29 Mar 2023 16:10:26 -0400 Subject: [PATCH 1/4] Add utility method to look for existing deployments --- utils/utils.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/utils/utils.go b/utils/utils.go index 8eea994c6..499f425ae 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" servingv1 "knative.dev/serving/pkg/apis/serving/v1" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) var APIVersionNotFoundError = errors.New("APIVersion is not available") @@ -1570,3 +1571,49 @@ func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.C return issuer.ResourceVersion, nil } } + +func CheckForExistingDeployments(operator string, name string, namespace string, client client.Client, request reconcile.Request, isKnativeSupported bool) error { + defaultMeta := metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + } + // Check to see if the knative service is owned by us + if isKnativeSupported { + ksvc := &servingv1.Service{ObjectMeta: defaultMeta} + err := client.Get(context.TODO(), request.NamespacedName, ksvc) + if err == nil { + for _, element := range ksvc.OwnerReferences { + if element.Kind != operator { + message := "Existing Knative Service \"" + name + "\" is not managed by this operator. It is being managed by \"" + element.Kind + "\". Resolve the naming conflict." + err = errors.New(message) + return err + } + } + } + } + // Check to see if the existing deployment or statefulsets are owned by us + deploy := &appsv1.Deployment{ObjectMeta: defaultMeta} + err := client.Get(context.TODO(), request.NamespacedName, deploy) + if err == nil { + for _, element := range deploy.OwnerReferences { + if element.Kind != operator { + message := "Existing Deployment \"" + name + "\" is not managed by this operator. It is being managed by \"" + element.Kind + "\". Resolve the naming conflict." + err = errors.New(message) + return err + } + } + } + // Check to see if the existing statefulset is owned by us + statefulSet := &appsv1.StatefulSet{ObjectMeta: defaultMeta} + err = client.Get(context.TODO(), request.NamespacedName, statefulSet) + if err == nil { + for _, element := range statefulSet.OwnerReferences { + if element.Kind != operator { + message := "Existing StatefulSet \"" + name + "\" is not managed by this operator. It is being managed by \"" + element.Kind + "\". Resolve the naming conflict." + err = errors.New(message) + return err + } + } + } + return nil +} From 671d91fff45bc97996a66891c659882bd7ad58a0 Mon Sep 17 00:00:00 2001 From: Scott McClements <31139144+smcclem@users.noreply.github.com> Date: Wed, 29 Mar 2023 16:29:33 -0400 Subject: [PATCH 2/4] Add previous deployments check --- controllers/runtimecomponent_controller.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/controllers/runtimecomponent_controller.go b/controllers/runtimecomponent_controller.go index 50999bb36..7fd1d40f4 100644 --- a/controllers/runtimecomponent_controller.go +++ b/controllers/runtimecomponent_controller.go @@ -234,6 +234,13 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req reqLogger.V(1).Info(fmt.Sprintf("%s is not supported on the cluster", servingv1.SchemeGroupVersion.String())) } + // Check if there is an existing Deployment, Statefulset or Knative service by this name + // not managed by this operator + err = appstacksutils.CheckForExistingDeployments("RuntimeComponent", instance.Name, instance.Namespace, r.GetClient(), req, isKnativeSupported) + if err != nil { + return r.ManageError(err, common.StatusConditionTypeReconciled, instance) + } + if instance.Spec.CreateKnativeService != nil && *instance.Spec.CreateKnativeService { // Clean up non-Knative resources resources := []client.Object{ From f71feec868dd1d969968a85b957c32e3a3a59455 Mon Sep 17 00:00:00 2001 From: Scott McClements <31139144+smcclem@users.noreply.github.com> Date: Fri, 31 Mar 2023 12:40:33 -0400 Subject: [PATCH 3/4] Update dev.sh bundle push --- scripts/dev.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/dev.sh b/scripts/dev.sh index 2b482ace8..da1a11144 100755 --- a/scripts/dev.sh +++ b/scripts/dev.sh @@ -268,7 +268,7 @@ bundle() { echo "------------" echo "bundle-push" echo "------------" - make -C $MAKEFILE_DIR bundle-push PUBLISH_REGISTRY=$OCP_REGISTRY_URL BUNDLE_IMG=$NAMESPACE/$OPERATOR_NAME-bundle:$VVERSION TLS_VERIFY=false + make -C $MAKEFILE_DIR bundle-push VERSION=$VVERSION IMG=$IMG IMAGE_TAG_BASE=$IMAGE_TAG_BASE BUNDLE_IMG=$BUNDLE_IMG CATALOG_IMG=$CATALOG_IMG TLS_VERIFY=false } ################################### From 66812bfde4a0c249b6318f9735cf4ab35c916e55 Mon Sep 17 00:00:00 2001 From: Scott McClements <31139144+smcclem@users.noreply.github.com> Date: Fri, 31 Mar 2023 12:41:42 -0400 Subject: [PATCH 4/4] Code review updates --- controllers/runtimecomponent_controller.go | 28 +++++++++++----------- utils/utils.go | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/controllers/runtimecomponent_controller.go b/controllers/runtimecomponent_controller.go index 7fd1d40f4..0fe2ffed9 100644 --- a/controllers/runtimecomponent_controller.go +++ b/controllers/runtimecomponent_controller.go @@ -135,6 +135,20 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req return reconcile.Result{}, err } + isKnativeSupported, err := r.IsGroupVersionSupported(servingv1.SchemeGroupVersion.String(), "Service") + if err != nil { + r.ManageError(err, common.StatusConditionTypeReconciled, instance) + } else if !isKnativeSupported { + reqLogger.V(1).Info(fmt.Sprintf("%s is not supported on the cluster", servingv1.SchemeGroupVersion.String())) + } + + // Check if there is an existing Deployment, Statefulset or Knative service by this name + // not managed by this operator + err = appstacksutils.CheckForNameConflicts("RuntimeComponent", instance.Name, instance.Namespace, r.GetClient(), req, isKnativeSupported) + if err != nil { + return r.ManageError(err, common.StatusConditionTypeReconciled, instance) + } + // initialize the RuntimeComponent instance instance.Initialize() _, err = appstacksutils.Validate(instance) @@ -227,20 +241,6 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req return r.ManageError(saErr, common.StatusConditionTypeReconciled, instance) } - isKnativeSupported, err := r.IsGroupVersionSupported(servingv1.SchemeGroupVersion.String(), "Service") - if err != nil { - r.ManageError(err, common.StatusConditionTypeReconciled, instance) - } else if !isKnativeSupported { - reqLogger.V(1).Info(fmt.Sprintf("%s is not supported on the cluster", servingv1.SchemeGroupVersion.String())) - } - - // Check if there is an existing Deployment, Statefulset or Knative service by this name - // not managed by this operator - err = appstacksutils.CheckForExistingDeployments("RuntimeComponent", instance.Name, instance.Namespace, r.GetClient(), req, isKnativeSupported) - if err != nil { - return r.ManageError(err, common.StatusConditionTypeReconciled, instance) - } - if instance.Spec.CreateKnativeService != nil && *instance.Spec.CreateKnativeService { // Clean up non-Knative resources resources := []client.Object{ diff --git a/utils/utils.go b/utils/utils.go index 499f425ae..9b508cfac 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1572,7 +1572,7 @@ func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.C } } -func CheckForExistingDeployments(operator string, name string, namespace string, client client.Client, request reconcile.Request, isKnativeSupported bool) error { +func CheckForNameConflicts(operator string, name string, namespace string, client client.Client, request reconcile.Request, isKnativeSupported bool) error { defaultMeta := metav1.ObjectMeta{ Name: name, Namespace: namespace,