From f094ebb044f9d9f077698199bc073f651f3c9265 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Tue, 14 May 2024 19:55:03 -0700 Subject: [PATCH] Cleanups Signed-off-by: Varsha Prasad Narsing --- api/v1alpha1/clusterextension_types.go | 24 ++++------- .../clusterextension_controller.go | 41 ------------------- .../clusterextension_controller_test.go | 37 ----------------- test/e2e/cluster_extension_install_test.go | 2 +- 4 files changed, 10 insertions(+), 94 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index ff8c36c6c..2c39e5e04 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -85,7 +85,6 @@ const ( TypeInstalled = "Installed" TypeResolved = "Resolved" TypeHasValidBundle = "HasValidBundle" - TypeHealthy = "Healthy" // TypeDeprecated is a rollup condition that is present when // any of the deprecated conditions are present. @@ -94,20 +93,19 @@ const ( TypeChannelDeprecated = "ChannelDeprecated" TypeBundleDeprecated = "BundleDeprecated" - ReasonErrorGettingClient = "ErrorGettingClient" - ReasonBundleLoadFailed = "BundleLoadFailed" - ReasonBundleLookupFailed = "BundleLookupFailed" + ReasonErrorGettingClient = "ErrorGettingClient" + ReasonBundleLoadFailed = "BundleLoadFailed" + ReasonInstallationFailed = "InstallationFailed" ReasonInstallationStatusUnknown = "InstallationStatusUnknown" ReasonInstallationSucceeded = "InstallationSucceeded" - ReasonInvalidSpec = "InvalidSpec" ReasonResolutionFailed = "ResolutionFailed" - ReasonResolutionUnknown = "ResolutionUnknown" - ReasonSuccess = "Success" - ReasonDeprecated = "Deprecated" - ReasonErrorGettingReleaseState = "ErrorGettingReleaseState" - ReasonUpgradeFailed = "UpgradeFailed" - ReasonCreateDynamicWatchFailed = "CreateDynamicWatchFailed" + + ReasonSuccess = "Success" + ReasonDeprecated = "Deprecated" + ReasonErrorGettingReleaseState = "ErrorGettingReleaseState" + ReasonUpgradeFailed = "UpgradeFailed" + ReasonCreateDynamicWatchFailed = "CreateDynamicWatchFailed" ) func init() { @@ -116,7 +114,6 @@ func init() { TypeInstalled, TypeResolved, TypeHasValidBundle, - TypeHealthy, TypeDeprecated, TypePackageDeprecated, TypeChannelDeprecated, @@ -126,11 +123,8 @@ func init() { conditionsets.ConditionReasons = append(conditionsets.ConditionReasons, ReasonInstallationSucceeded, ReasonResolutionFailed, - ReasonResolutionUnknown, - ReasonBundleLookupFailed, ReasonInstallationFailed, ReasonInstallationStatusUnknown, - ReasonInvalidSpec, ReasonSuccess, ReasonDeprecated, ReasonErrorGettingReleaseState, diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 4183dc9a1..b6e33d2fc 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -47,7 +47,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -554,46 +553,6 @@ func (r *ClusterExtensionReconciler) generateBundleDeploymentForUnpack(bundlePat } } -func (r *ClusterExtensionReconciler) GenerateExpectedBundleDeployment(o ocv1alpha1.ClusterExtension, bundlePath string, bundleProvisioner string) *unstructured.Unstructured { - // We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver. - // If you use a typed object, any default values from that struct get serialized into the JSON patch, which could - // cause unrelated fields to be patched back to the default value even though that isn't the intention. Using an - // unstructured ensures that the patch contains only what is specified. Using unstructured like this is basically - // identical to "kubectl apply -f" - - spec := map[string]interface{}{ - "installNamespace": o.Spec.InstallNamespace, - "provisionerClassName": bundleProvisioner, - "source": map[string]interface{}{ - // TODO: Don't assume image type - "type": string(rukpakv1alpha2.SourceTypeImage), - "image": map[string]interface{}{ - "ref": bundlePath, - }, - }, - } - - bd := &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": rukpakv1alpha2.GroupVersion.String(), - "kind": rukpakv1alpha2.BundleDeploymentKind, - "metadata": map[string]interface{}{ - "name": o.GetName(), - }, - "spec": spec, - }} - bd.SetOwnerReferences([]metav1.OwnerReference{ - { - APIVersion: ocv1alpha1.GroupVersion.String(), - Kind: "ClusterExtension", - Name: o.Name, - UID: o.UID, - Controller: ptr.To(true), - BlockOwnerDeletion: ptr.To(true), - }, - }) - return bd -} - // SetupWithManager sets up the controller with the Manager. func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error { controller, err := ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index b1206a0d9..1913dea5b 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" featuregatetesting "k8s.io/component-base/featuregate/testing" @@ -478,42 +477,6 @@ func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) } } -func TestGeneratedBundleDeployment(t *testing.T) { - test := []struct { - name string - clusterExtension ocv1alpha1.ClusterExtension - bundlePath string - bundleProvisioner string - }{ - { - name: "when all the specs are provided.", - clusterExtension: ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-bd", - UID: types.UID("test"), - }, - Spec: ocv1alpha1.ClusterExtensionSpec{ - InstallNamespace: "test-ns", - }, - }, - bundlePath: "testpath", - bundleProvisioner: "foo", - }, - } - - for _, tt := range test { - fakeReconciler := &controllers.ClusterExtensionReconciler{} - objUnstructured := fakeReconciler.GenerateExpectedBundleDeployment(tt.clusterExtension, tt.bundlePath, tt.bundleProvisioner) - resultBundleDeployment := &rukpakv1alpha2.BundleDeployment{} - require.NoError(t, runtime.DefaultUnstructuredConverter.FromUnstructured(objUnstructured.Object, resultBundleDeployment)) - // Verify the fields that have are being taken from cluster extension. - require.Equal(t, tt.clusterExtension.GetName(), resultBundleDeployment.GetName()) - require.Equal(t, tt.bundlePath, resultBundleDeployment.Spec.Source.Image.Ref) - require.Equal(t, tt.bundleProvisioner, resultBundleDeployment.Spec.ProvisionerClassName) - require.Equal(t, tt.clusterExtension.Spec.InstallNamespace, resultBundleDeployment.Spec.InstallNamespace) - } -} - func TestClusterExtensionUpgrade(t *testing.T) { cl, reconciler := newClientAndReconciler(t) ctx := context.Background() diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index c1c64b240..f73dac6a7 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -83,7 +83,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) - assert.Len(ct, clusterExtension.Status.Conditions, 9) + assert.Len(ct, clusterExtension.Status.Conditions, 8) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) if !assert.NotNil(ct, cond) { return