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

[SURE-9137] Add template errors to bundle and gitrepo status #3114

Merged
merged 3 commits into from
Jan 9, 2025
Merged
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
24 changes: 24 additions & 0 deletions e2e/assets/status/chart-with-template-vars/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
apiVersion: v2
name: chart-with-template-vars
description: A Helm chart for Kubernetes

# A chart can be either an 'application' or a 'library' chart.
#
# Application charts are a collection of templates that can be packaged into versioned archives
# to be deployed.
#
# Library charts provide useful utilities or functions for the chart developer. They're included as
# a dependency of application charts to inject those utilities and functions into the rendering
# pipeline. Library charts do not define any templates and therefore cannot be deployed.
type: application

# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.1.0

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
# It is recommended to use it with quotes.
appVersion: "1.16.0"
4 changes: 4 additions & 0 deletions e2e/assets/status/chart-with-template-vars/fleet.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
helm:
values:
templatedLabel: "${ .ClusterLabels.foo }-foo"
releaseName: reproducer
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: ConfigMap
apiVersion: v1
metadata:
name: chart-with-template-vars-configmap
namespace: fleet-local
data:
foo: bar
11 changes: 11 additions & 0 deletions e2e/assets/status/gitrepo.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
kind: GitRepo
apiVersion: fleet.cattle.io/v1alpha1
metadata:
name: {{.Name}}
namespace: fleet-local
spec:
repo: {{.Repo}}
branch: {{.Branch}}
targetNamespace: {{.TargetNamespace}}
paths:
- examples
8 changes: 4 additions & 4 deletions e2e/single-cluster/gitrepo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ var _ = Describe("Monitoring Git repos via HTTP for change", Label("infra-setup"
},
}
Eventually(func(g Gomega) {
status := getGitRepoStatus(k, gitrepoName)
status := getGitRepoStatus(g, k, gitrepoName)
g.Expect(status).To(matchGitRepoStatus(expectedStatus))
}).Should(Succeed())

Expand Down Expand Up @@ -355,7 +355,7 @@ var _ = Describe("Monitoring Git repos via HTTP for change", Label("infra-setup"
},
}
Eventually(func(g Gomega) {
status := getGitRepoStatus(k, gitrepoName)
status := getGitRepoStatus(g, k, gitrepoName)
g.Expect(status).To(matchGitRepoStatus(expectedStatus))

}).Should(Succeed())
Expand All @@ -381,10 +381,10 @@ func replace(path string, s string, r string) {
}

// getGitRepoStatus retrieves the status of the gitrepo with the provided name.
func getGitRepoStatus(k kubectl.Command, name string) fleet.GitRepoStatus {
func getGitRepoStatus(g Gomega, k kubectl.Command, name string) fleet.GitRepoStatus {
gr, err := k.Get("gitrepo", name, "-o=json")

Expect(err).ToNot(HaveOccurred())
g.Expect(err).ToNot(HaveOccurred())

var gitrepo fleet.GitRepo
_ = json.Unmarshal([]byte(gr), &gitrepo)
Expand Down
168 changes: 168 additions & 0 deletions e2e/single-cluster/status_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
package singlecluster_test

import (
"encoding/json"
"errors"
"fmt"
"math/rand"
"os"
"path"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/rancher/fleet/e2e/testenv"
"github.com/rancher/fleet/e2e/testenv/githelper"
"github.com/rancher/fleet/e2e/testenv/kubectl"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
"github.com/rancher/wrangler/v3/pkg/genericcondition"
corev1 "k8s.io/api/core/v1"
)

var _ = Describe("Checks status updates happen for a simple deployment", Ordered, func() {
Expand Down Expand Up @@ -108,3 +118,161 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered
})
})
})

var _ = Describe("Checks that template errors are shown in bundles and gitrepos", Ordered, Label("infra-setup"), func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What prevents these tests from being integration tests instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not actually sure that there is something that would prevent that, I was just worried about the complexity of having to work with several controllers in integration tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

We already support multiple reconcilers in integration tests ;)

var (
tmpDir string
cloneDir string
k kubectl.Command
gh *githelper.Git
repoName string
inClusterRepoURL string
gitrepoName string
r = rand.New(rand.NewSource(GinkgoRandomSeed()))
targetNamespace string
)

BeforeEach(func() {
k = env.Kubectl.Namespace(env.Namespace)
repoName = "repo"
})

JustBeforeEach(func() {
// Build git repo URL reachable _within_ the cluster, for the GitRepo
host, err := githelper.BuildGitHostname(env.Namespace)
Expect(err).ToNot(HaveOccurred())

addr, err := githelper.GetExternalRepoAddr(env, port, repoName)
Expect(err).ToNot(HaveOccurred())
gh = githelper.NewHTTP(addr)

inClusterRepoURL = gh.GetInClusterURL(host, port, repoName)

tmpDir, _ = os.MkdirTemp("", "fleet-")
cloneDir = path.Join(tmpDir, repoName)

gitrepoName = testenv.RandomFilename("status-test", r)

_, err = gh.Create(cloneDir, testenv.AssetPath("status/chart-with-template-vars"), "examples")
Expect(err).ToNot(HaveOccurred())

err = testenv.ApplyTemplate(k, testenv.AssetPath("status/gitrepo.yaml"), struct {
Name string
Repo string
Branch string
TargetNamespace string
}{
gitrepoName,
inClusterRepoURL,
gh.Branch,
targetNamespace, // to avoid conflicts with other tests
})
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
_ = os.RemoveAll(tmpDir)

_, err := k.Delete("gitrepo", gitrepoName)
Expect(err).ToNot(HaveOccurred())

// Check that the bundle deployment resource has been deleted
Eventually(func(g Gomega) {
out, _ := k.Get(
"bundledeployments",
"-A",
"-l",
fmt.Sprintf("fleet.cattle.io/repo-name=%s", gitrepoName),
)
g.Expect(out).To(ContainSubstring("No resources found"))
}).Should(Succeed())

// Deleting the targetNamespace is not necessary when the GitRepo did not successfully
// render, as in a few test cases here. If no targetNamespace was created, trying to delete
// the namespace will result in an error, which is why we are not checking for errors when
// deleting namespaces here.
_, _ = k.Delete("ns", targetNamespace)
})

expectNoError := func(g Gomega, conditions []genericcondition.GenericCondition) {
for _, condition := range conditions {
if condition.Type == string(fleet.Ready) {
g.Expect(condition.Status).To(Equal(corev1.ConditionTrue))
g.Expect(condition.Message).To(BeEmpty())
break
}
}
}

expectTargetingError := func(g Gomega, conditions []genericcondition.GenericCondition) {
found := false
for _, condition := range conditions {
if condition.Type == string(fleet.Ready) {
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
g.Expect(condition.Message).To(ContainSubstring("Targeting error"))
g.Expect(condition.Message).To(
ContainSubstring(
"<.ClusterLabels.foo>: map has no entry for key \"foo\""))
found = true
break
}
}
g.Expect(found).To(BeTrue())
}

ensureClusterHasLabelFoo := func() (string, error) {
return k.Namespace("fleet-local").
Patch("cluster", "local", "--type", "json", "--patch",
`[{"op": "add", "path": "/metadata/labels/foo", "value": "bar"}]`)
}

ensureClusterHasNoLabelFoo := func() (string, error) {
return k.Namespace("fleet-local").
Patch("cluster", "local", "--type", "json", "--patch",
`[{"op": "remove", "path": "/metadata/labels/foo"}]`)
}

When("a git repository is created that contains a template error", func() {
BeforeEach(func() {
targetNamespace = testenv.NewNamespaceName("target", r)
})

It("should have an error in the bundle", func() {
_, _ = ensureClusterHasNoLabelFoo()
Eventually(func(g Gomega) {
status := getBundleStatus(g, k, gitrepoName+"-examples")
expectTargetingError(g, status.Conditions)
}).Should(Succeed())
})

It("should have an error in the gitrepo", func() {
_, _ = ensureClusterHasNoLabelFoo()
Eventually(func(g Gomega) {
status := getGitRepoStatus(g, k, gitrepoName)
expectTargetingError(g, status.Conditions)
}).Should(Succeed())
})
})

When("a git repository is created that contains no template error", func() {
It("should have no error in the bundle", func() {
_, _ = ensureClusterHasLabelFoo()
Eventually(func(g Gomega) {
status := getBundleStatus(g, k, gitrepoName+"-examples")
expectNoError(g, status.Conditions)
weyfonk marked this conversation as resolved.
Show resolved Hide resolved
}).Should(Succeed())
})
})
})

// getBundleStatus retrieves the status of the bundle with the provided name.
func getBundleStatus(g Gomega, k kubectl.Command, name string) fleet.BundleStatus {
gr, err := k.Get("bundle", name, "-o=json")

g.Expect(err).ToNot(HaveOccurred())

var bundle fleet.Bundle
_ = json.Unmarshal([]byte(gr), &bundle)

return bundle.Status
}
Loading
Loading