From f8eb384b2f44cb86b0badea7b0261a854427b9d4 Mon Sep 17 00:00:00 2001 From: Frederic Wilhelm Date: Wed, 23 Oct 2024 10:47:14 +0200 Subject: [PATCH 1/3] create BeforeEach-func for components --- .../resource/resource_controller_test.go | 59 +++++++++---------- internal/controller/resource/suite_test.go | 10 ++++ 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/internal/controller/resource/resource_controller_test.go b/internal/controller/resource/resource_controller_test.go index 4afc87bc..9af2c6b7 100644 --- a/internal/controller/resource/resource_controller_test.go +++ b/internal/controller/resource/resource_controller_test.go @@ -55,7 +55,6 @@ import ( const ( CTFPath = "ocm-k8s-ctfstore--*" Namespace = "test-namespace" - RepositoryObj = "test-repository" Component = "ocm.software/test-component" ComponentObj = "test-component" ComponentVersion = "1.0.0" @@ -85,17 +84,19 @@ var _ = Describe("Resource Controller", func() { }) Context("resource controller", func() { - It("can reconcile a resource", func() { - By("creating namespace object") - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: Namespace, - }, - } - Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) + var ( + componentName string + testNumber int + //componentObj *v1alpha1.Component + ) + + BeforeEach(func() { + By("mocking the component controller") + componentName = fmt.Sprintf("%s-%d", ComponentObj, testNumber) + mockComponentReconciler(ctx, env, ctfPath, componentName) + }) - By("preparing a mock component") - prepareComponent(ctx, env, ctfPath) + It("can reconcile a resource", func() { By("creating a resource object") resource := &v1alpha1.Resource{ @@ -105,7 +106,7 @@ var _ = Describe("Resource Controller", func() { }, Spec: v1alpha1.ResourceSpec{ ComponentRef: corev1.LocalObjectReference{ - Name: ComponentObj, + Name: componentName, }, Resource: v1alpha1.ResourceID{ ByReference: v1alpha1.ResourceReference{ @@ -155,10 +156,8 @@ var _ = Describe("Resource Controller", func() { }) }) -// prepareComponent essentially mocks the behavior of the component reconciler to provider the necessary component and -// artifact for the resource controller. -func prepareComponent(ctx context.Context, env *Builder, ctfPath string) { - By("creating ocm repositories with a component and resource") +func mockComponentReconciler(ctx context.Context, env *Builder, ctfPath, name string) { + // Create a ctf storing a component-version with a blob data as content env.OCMCommonTransport(ctfPath, accessio.FormatDirectory, func() { env.Component(Component, func() { env.Version(ComponentVersion, func() { @@ -169,27 +168,28 @@ func prepareComponent(ctx context.Context, env *Builder, ctfPath string) { }) }) - By("creating a component descriptor") - tmpDirCd := Must(os.MkdirTemp("/tmp", "descriptors-")) + // Creating component descriptor + tmp := Must(os.MkdirTemp("/tmp", "descriptors-")) DeferCleanup(func() error { - return os.RemoveAll(tmpDirCd) + return os.RemoveAll(tmp) }) + repo := Must(ctf.Open(env, accessobj.ACC_WRITABLE, ctfPath, vfs.FileMode(vfs.O_RDWR), env)) cv := Must(repo.LookupComponentVersion(Component, ComponentVersion)) cd := Must(ocm.ListComponentDescriptors(ctx, cv, repo)) dataCds := Must(yaml.Marshal(cd)) - MustBeSuccessful(os.WriteFile(filepath.Join(tmpDirCd, v1alpha1.OCMComponentDescriptorList), dataCds, 0o655)) + MustBeSuccessful(os.WriteFile(filepath.Join(tmp, v1alpha1.OCMComponentDescriptorList), dataCds, 0o655)) - By("creating a component object") + // Create a component object component := &v1alpha1.Component{ ObjectMeta: metav1.ObjectMeta{ Namespace: Namespace, - Name: ComponentObj, + Name: name, }, Spec: v1alpha1.ComponentSpec{ RepositoryRef: v1alpha1.ObjectKey{ - Namespace: Namespace, - Name: RepositoryObj, + Namespace: "some-namespace", + Name: "some-name", }, Component: Component, Semver: ComponentVersion, @@ -198,13 +198,12 @@ func prepareComponent(ctx context.Context, env *Builder, ctfPath string) { } Expect(k8sClient.Create(ctx, component)).To(Succeed()) - By("creating an component artifact") - revision := ComponentObj + "-" + ComponentVersion + // Create a component artifact object var artifactName string - Expect(globStorage.ReconcileArtifact(ctx, component, revision, tmpDirCd, revision+".tar.gz", + Expect(globStorage.ReconcileArtifact(ctx, component, "revision", tmp, "component-descriptor.tar.gz", func(art *artifactv1.Artifact, _ string) error { // Archive directory to storage - if err := globStorage.Archive(art, tmpDirCd, nil); err != nil { + if err := globStorage.Archive(art, tmp, nil); err != nil { return fmt.Errorf("unable to archive artifact to storage: %w", err) } @@ -214,7 +213,7 @@ func prepareComponent(ctx context.Context, env *Builder, ctfPath string) { }, )).To(Succeed()) - By("checking that the artifact has been created successfully") + // Create a component artifact object artifact := &artifactv1.Artifact{ ObjectMeta: metav1.ObjectMeta{ Name: artifactName, @@ -223,7 +222,7 @@ func prepareComponent(ctx context.Context, env *Builder, ctfPath string) { } Eventually(komega.Get(artifact)).Should(Succeed()) - By("updating the component object with the respective status") + // Update the component object status baseComponent := component.DeepCopy() ready := *conditions.TrueCondition("Ready", "ready", "message") ready.LastTransitionTime = metav1.Time{Time: time.Now()} diff --git a/internal/controller/resource/suite_test.go b/internal/controller/resource/suite_test.go index 5149834f..5b16ffcd 100644 --- a/internal/controller/resource/suite_test.go +++ b/internal/controller/resource/suite_test.go @@ -28,6 +28,8 @@ import ( artifactv1 "github.com/openfluxcd/artifact/api/v1alpha1" "github.com/openfluxcd/controller-manager/server" "github.com/openfluxcd/controller-manager/storage" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" @@ -127,6 +129,14 @@ var _ = BeforeSuite(func() { ctx, cancel := context.WithCancel(context.Background()) DeferCleanup(cancel) + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: Namespace, + }, + } + Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) + go func() { defer GinkgoRecover() Expect(artifactServer.Start(ctx)).To(Succeed()) From 570483471dd7d8843071fb36d57a6bb1604a8b63 Mon Sep 17 00:00:00 2001 From: Frederic Wilhelm Date: Wed, 23 Oct 2024 11:14:33 +0200 Subject: [PATCH 2/3] remove unused test code --- internal/controller/component/component_controller_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/controller/component/component_controller_test.go b/internal/controller/component/component_controller_test.go index 623c0746..1d0b3774 100644 --- a/internal/controller/component/component_controller_test.go +++ b/internal/controller/component/component_controller_test.go @@ -115,12 +115,6 @@ var _ = Describe("Component Controller", func() { testNumber++ }) - AfterEach(func() { - // make sure the repo is still ready - conditions.MarkTrue(repositoryObj, "Ready", "ready", "message") - Expect(k8sClient.Status().Update(ctx, repositoryObj)).To(Succeed()) - }) - It("reconcileComponent a component", func() { By("creating a component") component := &v1alpha1.Component{ From c34ddd11da359ff7165f76f54e2eb52c37960803 Mon Sep 17 00:00:00 2001 From: Frederic Wilhelm Date: Tue, 29 Oct 2024 18:18:37 +0100 Subject: [PATCH 3/3] wip --- api/v1alpha1/condition_types.go | 3 + internal/controller/component/suite_test.go | 2 +- .../configuration/configuration_controller.go | 2 + .../resource/resource_controller.go | 31 +++----- .../resource/resource_controller_test.go | 75 +++++++++++++++---- internal/controller/resource/suite_test.go | 38 +++++++++- 6 files changed, 113 insertions(+), 38 deletions(-) diff --git a/api/v1alpha1/condition_types.go b/api/v1alpha1/condition_types.go index 46b252b8..7985df79 100644 --- a/api/v1alpha1/condition_types.go +++ b/api/v1alpha1/condition_types.go @@ -39,6 +39,9 @@ const ( // RepositoryIsNotReadyReason is used when the referenced repository is not Ready yet. RepositoryIsNotReadyReason = "RepositoryIsNotReady" + // ComponentNotFoundReason is used when the referenced component is not found. + ComponentNotFoundReason = "ComponentNotFound" + // ComponentIsNotReadyReason is used when the referenced component is not Ready yet. ComponentIsNotReadyReason = "ComponentIsNotReady" diff --git a/internal/controller/component/suite_test.go b/internal/controller/component/suite_test.go index d2018033..1a111428 100644 --- a/internal/controller/component/suite_test.go +++ b/internal/controller/component/suite_test.go @@ -51,7 +51,7 @@ import ( const ( ARTIFACT_PATH = "ocm-k8s-artifactstore--*" - ARTIFACT_SERVER = "localhost:8080" + ARTIFACT_SERVER = "localhost:0" ) var cfg *rest.Config diff --git a/internal/controller/configuration/configuration_controller.go b/internal/controller/configuration/configuration_controller.go index a5ec32b5..6e039d7c 100644 --- a/internal/controller/configuration/configuration_controller.go +++ b/internal/controller/configuration/configuration_controller.go @@ -85,6 +85,7 @@ type Reconciler struct { // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, err error) { + log.FromContext(ctx).V(1).Info("reconciling") configuration := &v1alpha1.ConfiguredResource{} if err := r.Get(ctx, req.NamespacedName, configuration); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) @@ -228,6 +229,7 @@ func (r *Reconciler) reconcileExists(ctx context.Context, configuration *v1alpha return ctrl.Result{}, fmt.Errorf("failed to reconcile artifact: %w", err) } + logger.Info("updating") logger.Info("configuration successful", "artifact", configuration.Status.ArtifactRef) status.MarkReady(r.EventRecorder, configuration, "configured successfully") diff --git a/internal/controller/resource/resource_controller.go b/internal/controller/resource/resource_controller.go index 58870f79..9160e799 100644 --- a/internal/controller/resource/resource_controller.go +++ b/internal/controller/resource/resource_controller.go @@ -123,19 +123,16 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { // +kubebuilder:rbac:groups=openfluxcd.mandelsoft.org,resources=artifacts/finalizers,verbs=update func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log.FromContext(ctx).V(1).Info("starting reconciling resource") + resource := &v1alpha1.Resource{} if err := r.Get(ctx, req.NamespacedName, resource); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - return r.reconcileWithStatusUpdate(ctx, resource) -} - -func (r *Reconciler) reconcileWithStatusUpdate(ctx context.Context, resource *v1alpha1.Resource) (ctrl.Result, error) { patchHelper := patch.NewSerialPatcher(resource, r.Client) - result, err := r.reconcileExists(ctx, resource) - + result, err := r.reconcilePrerequisite(ctx, resource) err = errors.Join(err, status.UpdateStatus(ctx, patchHelper, resource, r.EventRecorder, resource.GetRequeueAfter(), err)) if err != nil { return ctrl.Result{}, err @@ -144,9 +141,9 @@ func (r *Reconciler) reconcileWithStatusUpdate(ctx context.Context, resource *v1 return result, nil } -func (r *Reconciler) reconcileExists(ctx context.Context, resource *v1alpha1.Resource) (ctrl.Result, error) { +func (r *Reconciler) reconcilePrerequisite(ctx context.Context, resource *v1alpha1.Resource) (ctrl.Result, error) { logger := log.FromContext(ctx) - logger.V(1).Info("preparing reconciling resource") + logger.V(1).Info("checking if resource is suspended or deleted") if resource.Spec.Suspend { return ctrl.Result{}, nil @@ -159,6 +156,7 @@ func (r *Reconciler) reconcileExists(ctx context.Context, resource *v1alpha1.Res } if removed := controllerutil.RemoveFinalizer(resource, v1alpha1.ArtifactFinalizer); removed { + logger.V(1).Info("removing finalizer, restarting reconciler") if err := r.Update(ctx, resource); err != nil { return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) } @@ -168,19 +166,16 @@ func (r *Reconciler) reconcileExists(ctx context.Context, resource *v1alpha1.Res } if added := controllerutil.AddFinalizer(resource, v1alpha1.ArtifactFinalizer); added { - err := r.Update(ctx, resource) - if err != nil { + logger.V(1).Info("adding finalizer, restarting reconciler") + if err := r.Update(ctx, resource); err != nil { return ctrl.Result{}, fmt.Errorf("failed to add finalizer: %w", err) } return ctrl.Result{Requeue: true}, nil } - return r.reconcile(ctx, resource) -} + logger.V(1).Info("checking if referenced component is ready") -func (r *Reconciler) reconcile(ctx context.Context, resource *v1alpha1.Resource) (ctrl.Result, error) { - logger := log.FromContext(ctx) // Get component to resolve resource from component descriptor and verify digest component := &v1alpha1.Component{} if err := r.Get(ctx, types.NamespacedName{ @@ -203,13 +198,9 @@ func (r *Reconciler) reconcile(ctx context.Context, resource *v1alpha1.Resource) return ctrl.Result{}, errors.New("component is not ready") } - return r.reconcileOCM(ctx, resource, component) -} - -func (r *Reconciler) reconcileOCM(ctx context.Context, resource *v1alpha1.Resource, component *v1alpha1.Component) (ctrl.Result, error) { octx := ocmctx.New(datacontext.MODE_EXTENDED) - result, err := r.reconcileResource(ctx, octx, resource, component) + result, err := r.reconcile(ctx, octx, resource, component) // Always finalize ocm context after reconciliation err = errors.Join(err, octx.Finalize()) @@ -223,7 +214,7 @@ func (r *Reconciler) reconcileOCM(ctx context.Context, resource *v1alpha1.Resour return result, nil } -func (r *Reconciler) reconcileResource(ctx context.Context, octx ocmctx.Context, resource *v1alpha1.Resource, component *v1alpha1.Component) (ctrl.Result, error) { +func (r *Reconciler) reconcile(ctx context.Context, octx ocmctx.Context, resource *v1alpha1.Resource, component *v1alpha1.Component) (ctrl.Result, error) { logger := log.FromContext(ctx) logger.V(1).Info("reconciling resource") diff --git a/internal/controller/resource/resource_controller_test.go b/internal/controller/resource/resource_controller_test.go index 9af2c6b7..8380f877 100644 --- a/internal/controller/resource/resource_controller_test.go +++ b/internal/controller/resource/resource_controller_test.go @@ -35,6 +35,7 @@ import ( artifactv1 "github.com/openfluxcd/artifact/api/v1alpha1" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" . "ocm.software/ocm/api/helper/builder" environment "ocm.software/ocm/api/helper/env" @@ -65,11 +66,10 @@ const ( var _ = Describe("Resource Controller", func() { var ( - ctx context.Context - cancel context.CancelFunc env *Builder ctfPath string ) + BeforeEach(func() { ctfPath = Must(os.MkdirTemp("", CTFPath)) DeferCleanup(func() error { @@ -78,31 +78,29 @@ var _ = Describe("Resource Controller", func() { env = NewBuilder(environment.FileSystem(osfs.OsFs)) DeferCleanup(env.Cleanup) - - ctx, cancel = context.WithCancel(context.Background()) - DeferCleanup(cancel) }) Context("resource controller", func() { var ( componentName string + resourceName string testNumber int - //componentObj *v1alpha1.Component ) - BeforeEach(func() { + BeforeEach(func(ctx SpecContext) { By("mocking the component controller") componentName = fmt.Sprintf("%s-%d", ComponentObj, testNumber) + resourceName = fmt.Sprintf("%s-%d", ResourceObj, testNumber) mockComponentReconciler(ctx, env, ctfPath, componentName) + testNumber++ }) - It("can reconcile a resource", func() { - + FIt("can reconcile a resource", func(ctx SpecContext) { By("creating a resource object") resource := &v1alpha1.Resource{ ObjectMeta: metav1.ObjectMeta{ Namespace: Namespace, - Name: ResourceObj, + Name: resourceName, }, Spec: v1alpha1.ResourceSpec{ ComponentRef: corev1.LocalObjectReference{ @@ -117,26 +115,23 @@ var _ = Describe("Resource Controller", func() { }, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - DeferCleanup(func(ctx SpecContext) { - Expect(k8sClient.Delete(ctx, resource, client.PropagationPolicy(metav1.DeletePropagationForeground))).To(Succeed()) - }) By("checking that the resource has been reconciled successfully") - Eventually(komega.Object(resource), "5m").Should( + Eventually(komega.Object(resource), "15s", "100ms").Should( HaveField("Status.ObservedGeneration", Equal(int64(1)))) Expect(resource).To(HaveField("Status.ArtifactRef.Name", Not(BeEmpty()))) Expect(resource).To(HaveField("Status.Resource.Name", Equal(ResourceObj))) Expect(resource).To(HaveField("Status.Resource.Type", Equal(artifacttypes.PLAIN_TEXT))) Expect(resource).To(HaveField("Status.Resource.Version", Equal(ResourceVersion))) - By("checking that the artifact has been created successfully") + By("checking that the resource artifact was created") artifact := &artifactv1.Artifact{ ObjectMeta: metav1.ObjectMeta{ Namespace: resource.Namespace, Name: resource.Status.ArtifactRef.Name, }, } - Eventually(komega.Get(artifact)).Should(Succeed()) + Eventually(komega.Get(artifact), "15s", "100ms").Should(Succeed()) By("checking that the artifact server provides the resource") r := Must(http.Get(artifact.Spec.URL)) @@ -152,11 +147,59 @@ var _ = Describe("Resource Controller", func() { resourceContent := Must(io.ReadAll(reader)) Expect(string(resourceContent)).To(Equal(ResourceContent)) + + By("checking that the resource is deleted correctly") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + Eventually(komega.Object(resource), "15s").Should(HaveField("Status.DeletionTimestamp", Not(BeNil()))) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(resource), resource)).To(Succeed()) + + // Simulate ownerreference deletion + Expect(k8sClient.Delete(ctx, artifact)).To(Succeed()) + + Eventually(func(ctx SpecContext) error { + return k8sClient.Get(ctx, client.ObjectKeyFromObject(resource), resource) + }).WithContext(ctx).Should(Satisfy(errors.IsNotFound)) + }) + + It("stops when requirements are not met: component not found", func(ctx SpecContext) { + By("creating a resource object") + resource := &v1alpha1.Resource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: Namespace, + Name: resourceName, + }, + Spec: v1alpha1.ResourceSpec{ + ComponentRef: corev1.LocalObjectReference{ + Name: "anotherName", + }, + Resource: v1alpha1.ResourceID{ + ByReference: v1alpha1.ResourceReference{ + Resource: v1.NewIdentity(ResourceObj), + }, + }, + Interval: metav1.Duration{Duration: time.Minute * 5}, + }, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + DeferCleanup(func(ctx SpecContext) { + Expect(k8sClient.Delete(ctx, resource, client.PropagationPolicy(metav1.DeletePropagationForeground))).To(Succeed()) + }) + + By("checking that the resource was not reconciled successfully") + Eventually(komega.Object(resource), "5m").Should( + HaveField("Status.ObservedGeneration", Equal(int64(1)))) + Expect(resource).To(HaveField("Status.ArtifactRef.Name", Not(BeEmpty()))) + Expect(resource).To(HaveField("Status.Resource.Name", Equal(ResourceObj))) + Expect(resource).To(HaveField("Status.Resource.Type", Equal(artifacttypes.PLAIN_TEXT))) + Expect(resource).To(HaveField("Status.Resource.Version", Equal(ResourceVersion))) + }) }) }) func mockComponentReconciler(ctx context.Context, env *Builder, ctfPath, name string) { + GinkgoHelper() + // Create a ctf storing a component-version with a blob data as content env.OCMCommonTransport(ctfPath, accessio.FormatDirectory, func() { env.Component(Component, func() { diff --git a/internal/controller/resource/suite_test.go b/internal/controller/resource/suite_test.go index 5b16ffcd..dac65d29 100644 --- a/internal/controller/resource/suite_test.go +++ b/internal/controller/resource/suite_test.go @@ -32,6 +32,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -52,7 +54,7 @@ import ( const ( ARTIFACT_PATH = "ocm-k8s-artifactstore--*" - ARTIFACT_SERVER = "localhost:8081" + ARTIFACT_SERVER = "localhost:8085" ) var cfg *rest.Config @@ -91,6 +93,8 @@ var _ = BeforeSuite(func() { Expect(cfg).NotTo(BeNil()) DeferCleanup(testEnv.Stop) + createKubeConfig(cfg) + Expect(v1alpha1.AddToScheme(scheme.Scheme)).Should(Succeed()) Expect(artifactv1.AddToScheme(scheme.Scheme)).Should(Succeed()) Expect(err).NotTo(HaveOccurred()) @@ -146,3 +150,35 @@ var _ = BeforeSuite(func() { Expect(k8sManager.Start(ctx)).To(Succeed()) }() }) + +func createKubeConfig(cfg *rest.Config) { + // Write the kubeconfig to the temporary file + kubeconfig := api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: cfg.Host, + CertificateAuthorityData: cfg.CAData, + }, + }, + AuthInfos: map[string]*api.AuthInfo{ + "test-user": { + ClientCertificateData: cfg.CertData, + ClientKeyData: cfg.KeyData, + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "test-user", + }, + }, + CurrentContext: "test-context", + } + + kubeconfigFile, err := os.Create(filepath.Join(os.Getenv("HOME"), ".kubeconfig.env")) + + kubeconfigBytes, err := clientcmd.Write(kubeconfig) + Expect(err).NotTo(HaveOccurred()) + _, err = kubeconfigFile.Write(kubeconfigBytes) + Expect(err).NotTo(HaveOccurred()) +}