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

chore: add tests to the resource reconciler #47

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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
3 changes: 3 additions & 0 deletions api/v1alpha1/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
6 changes: 0 additions & 6 deletions internal/controller/component/component_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/component/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import (

const (
ARTIFACT_PATH = "ocm-k8s-artifactstore--*"
ARTIFACT_SERVER = "localhost:8080"
ARTIFACT_SERVER = "localhost:0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh?

)

var cfg *rest.Config
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")

Expand Down
31 changes: 11 additions & 20 deletions internal/controller/resource/resource_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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{
Expand All @@ -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())
Expand All @@ -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")

Expand Down
126 changes: 84 additions & 42 deletions internal/controller/resource/resource_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -55,7 +56,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"
Expand All @@ -66,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 {
Expand All @@ -79,33 +78,33 @@ 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() {
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
resourceName string
testNumber int
)

By("preparing a mock component")
prepareComponent(ctx, env, ctfPath)
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++
})

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{
Name: ComponentObj,
Name: componentName,
},
Resource: v1alpha1.ResourceID{
ByReference: v1alpha1.ResourceReference{
Expand All @@ -116,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))
Expand All @@ -151,14 +147,60 @@ 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)))

})
})
})

// 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) {
GinkgoHelper()

// 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() {
Expand All @@ -169,27 +211,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,
Expand All @@ -198,13 +241,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)
}

Expand All @@ -214,7 +256,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,
Expand All @@ -223,7 +265,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()}
Expand Down
Loading
Loading