Skip to content

Commit

Permalink
chore: shifts FeatureTracker creation to Feature's Apply phase (opend…
Browse files Browse the repository at this point in the history 
…atahub-io#795)
  • Loading branch information
@bartoszmajsak @zdtsw
bartoszmajsak authored and zdtsw committed Jan 15, 2024
1 parent 830c3eb commit bedf8c0
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 27 deletions.
6 changes: 0 additions & 6 deletions pkg/feature/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,6 @@ func (fb *featureBuilder) Load() (*Feature, error) {
}
}

if feature.Enabled {
if err := feature.createResourceTracker(); err != nil {
return feature, err
}
}

return feature, nil
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/feature/feature.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ func (f *Feature) Apply() error {
return nil
}

if err := f.createResourceTracker(); err != nil {
return err
}

// Verify all precondition and collect errors
var multiErr *multierror.Error
for _, precondition := range f.preconditions {
Expand Down Expand Up @@ -229,7 +233,9 @@ func (f *Feature) createResourceTracker() error {

// Register its own cleanup
f.addCleanup(func(feature *Feature) error {
if err := f.DynamicClient.Resource(gvr.ResourceTracker).Delete(context.TODO(), f.Spec.Tracker.Name, metav1.DeleteOptions{}); err != nil && !k8serrors.IsNotFound(err) {
err := f.DynamicClient.Resource(gvr.ResourceTracker).Delete(context.TODO(), f.Spec.Tracker.Name, metav1.DeleteOptions{})

if !k8serrors.IsNotFound(err) {
return err
}

Expand Down
43 changes: 23 additions & 20 deletions tests/integration/features/serverless_feature_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,16 @@ var _ = Describe("Serverless feature", func() {
Load()

Expect(err).ToNot(HaveOccurred())

// Creates the actual Feature instance so that associated FeatureTracker is created as well
Expect(testFeature.Apply()).To(Succeed())
})

Context("verifying preconditions", func() {

When("operator is not installed", func() {
It("operator presence check should return an error", func() {
Expect(serverless.EnsureServerlessOperatorInstalled(testFeature)).To(HaveOccurred())
Expect(serverless.EnsureServerlessOperatorInstalled(testFeature)).ToNot(Succeed())
})
})

Expand All @@ -99,10 +102,10 @@ var _ = Describe("Serverless feature", func() {
BeforeEach(func() {
// Create KNativeServing the CRD
knativeServingCrdObj = &apiextensionsv1.CustomResourceDefinition{}
Expect(yaml.Unmarshal([]byte(knativeServingCrd), knativeServingCrdObj)).ToNot(HaveOccurred())
Expect(yaml.Unmarshal([]byte(knativeServingCrd), knativeServingCrdObj)).To(Succeed())
c, err := client.New(envTest.Config, client.Options{})
Expect(err).ToNot(HaveOccurred())
Expect(c.Create(context.TODO(), knativeServingCrdObj)).ToNot(HaveOccurred())
Expect(c.Create(context.TODO(), knativeServingCrdObj)).To(Succeed())

crdOptions := envtest.CRDInstallOptions{PollInterval: interval, MaxTime: timeout}
err = envtest.WaitForCRDs(envTest.Config, []*apiextensionsv1.CustomResourceDefinition{knativeServingCrdObj}, crdOptions)
Expand All @@ -115,25 +118,25 @@ var _ = Describe("Serverless feature", func() {
})

It("operator presence check should succeed", func() {
Expect(serverless.EnsureServerlessOperatorInstalled(testFeature)).ToNot(HaveOccurred())
Expect(serverless.EnsureServerlessOperatorInstalled(testFeature)).To(Succeed())
})

It("KNative serving absence check should succeed if serving is not installed", func() {
Expect(serverless.EnsureServerlessAbsent(testFeature)).ToNot(HaveOccurred())
Expect(serverless.EnsureServerlessAbsent(testFeature)).To(Succeed())
})

It("KNative serving absence check should fail when serving is present", func() {
ns := envtestutil.AppendRandomNameTo(testNamespacePrefix)
nsResource := createNamespace(ns)
Expect(envTestClient.Create(context.TODO(), nsResource)).ToNot(HaveOccurred())
Expect(envTestClient.Create(context.TODO(), nsResource)).To(Succeed())
defer objectCleaner.DeleteAll(nsResource)

knativeServing := &unstructured.Unstructured{}
Expect(yaml.Unmarshal([]byte(knativeServingInstance), knativeServing)).ToNot(HaveOccurred())
Expect(yaml.Unmarshal([]byte(knativeServingInstance), knativeServing)).To(Succeed())
knativeServing.SetNamespace(nsResource.Name)
Expect(envTestClient.Create(context.TODO(), knativeServing)).ToNot(HaveOccurred())
Expect(envTestClient.Create(context.TODO(), knativeServing)).To(Succeed())

Expect(serverless.EnsureServerlessAbsent(testFeature)).To(HaveOccurred())
Expect(serverless.EnsureServerlessAbsent(testFeature)).ToNot(Succeed())
})
})
})
Expand Down Expand Up @@ -163,16 +166,16 @@ var _ = Describe("Serverless feature", func() {
Expect(yaml.Unmarshal([]byte(openshiftClusterIngress), osIngressResource)).ToNot(HaveOccurred())
c, err := client.New(envTest.Config, client.Options{})
Expect(err).ToNot(HaveOccurred())
Expect(c.Create(context.TODO(), osIngressResource)).ToNot(HaveOccurred())
Expect(c.Create(context.TODO(), osIngressResource)).To(Succeed())

// Default value is blank -> testFeature.Spec.Serving.IngressGateway.Domain = ""
Expect(serverless.ServingIngressDomain(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingIngressDomain(testFeature)).To(Succeed())
Expect(testFeature.Spec.KnativeIngressDomain).To(Equal("*.foo.io"))
})

It("should use user value when set in the DSCI", func() {
testFeature.Spec.Serving.IngressGateway.Domain = testDomainFooCom
Expect(serverless.ServingIngressDomain(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingIngressDomain(testFeature)).To(Succeed())
Expect(testFeature.Spec.KnativeIngressDomain).To(Equal(testDomainFooCom))
})
})
Expand All @@ -183,16 +186,16 @@ var _ = Describe("Serverless feature", func() {
It("should create a TLS secret if certificate is SelfSigned", func() {
ns := envtestutil.AppendRandomNameTo(testNamespacePrefix)
nsResource := createNamespace(ns)
Expect(envTestClient.Create(context.TODO(), nsResource)).ToNot(HaveOccurred())
Expect(envTestClient.Create(context.TODO(), nsResource)).To(Succeed())
defer objectCleaner.DeleteAll(nsResource)

testFeature.Spec.ControlPlane.Namespace = nsResource.Name
testFeature.Spec.Serving.IngressGateway.Certificate.Type = infrav1.SelfSigned
testFeature.Spec.Serving.IngressGateway.Domain = testDomainFooCom
Expect(serverless.ServingDefaultValues(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingIngressDomain(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingDefaultValues(testFeature)).To(Succeed())
Expect(serverless.ServingIngressDomain(testFeature)).To(Succeed())

Expect(serverless.ServingCertificateResource(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingCertificateResource(testFeature)).To(Succeed())

secret, err := testFeature.Clientset.CoreV1().Secrets(nsResource.Name).Get(context.TODO(), serverless.DefaultCertificateSecretName, v1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expand All @@ -202,16 +205,16 @@ var _ = Describe("Serverless feature", func() {
It("should not create any TLS secret if certificate is user provided", func() {
ns := envtestutil.AppendRandomNameTo(testNamespacePrefix)
nsResource := createNamespace(ns)
Expect(envTestClient.Create(context.TODO(), nsResource)).ToNot(HaveOccurred())
Expect(envTestClient.Create(context.TODO(), nsResource)).To(Succeed())
defer objectCleaner.DeleteAll(nsResource)

testFeature.Spec.ControlPlane.Namespace = nsResource.Name
testFeature.Spec.Serving.IngressGateway.Certificate.Type = infrav1.Provided
testFeature.Spec.Serving.IngressGateway.Domain = "*.foo.com"
Expect(serverless.ServingDefaultValues(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingIngressDomain(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingDefaultValues(testFeature)).To(Succeed())
Expect(serverless.ServingIngressDomain(testFeature)).To(Succeed())

Expect(serverless.ServingCertificateResource(testFeature)).ToNot(HaveOccurred())
Expect(serverless.ServingCertificateResource(testFeature)).To(Succeed())

list, err := testFeature.Clientset.CoreV1().Secrets(nsResource.Name).List(context.TODO(), v1.ListOptions{})
Expect(err).ToNot(HaveOccurred())
Expand Down

0 comments on commit bedf8c0

Please sign in to comment.