From c87b11d6c6979e1aadd4bbb214b19fb7b784dd04 Mon Sep 17 00:00:00 2001 From: nikimanoledaki Date: Fri, 28 Jan 2022 17:46:24 +0100 Subject: [PATCH] Add profiles.yaml to Kustomization for system/ dir - Improve acceptance test for Profiles service - Improve logic for creating kind clusters to avoid duplicate prefix e.g. kind-kind-x --- pkg/git/git.go | 8 +- pkg/models/manifest.go | 3 +- pkg/models/manifest_test.go | 2 +- pkg/services/install/install_test.go | 2 +- pkg/services/profiles/add.go | 2 +- pkg/services/profiles/add_test.go | 48 ++--- .../fakegitprovider/pull_request.go | 167 ++++++++++++++++++ test/acceptance/test/profiles_test.go | 69 +++----- test/acceptance/test/utils.go | 11 +- 9 files changed, 232 insertions(+), 80 deletions(-) create mode 100644 pkg/vendorfakes/fakegitprovider/pull_request.go diff --git a/pkg/git/git.go b/pkg/git/git.go index 041b9411c8a..f8b5aa6cfee 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -57,8 +57,8 @@ const WegoClusterOSWorkloadDir = "system" // WegoClusterUserWorkloadDir is where user workload manifests will live in the GitOps repo const WegoClusterUserWorkloadDir = "user" -// ManifestFileName contains the manifests of all installed Profiles -const ManifestFileName = "profiles.yaml" +// ProfilesManifestFileName contains the manifests of all installed Profiles +const ProfilesManifestFileName = "profiles.yaml" func getClusterPath(clusterName string) string { return filepath.Join(WegoRoot, WegoClusterDir, clusterName) @@ -77,10 +77,10 @@ func GetSystemQualifiedPath(clusterName string, relativePath string) string { return filepath.Join(GetSystemPath(clusterName), relativePath) } -// GetProfilesPath returns the path of the file containing the manifests of installed Profile manifests +// GetProfilesPath returns the path of the file containing the manifests of installed Profiles // joined with the cluster's system directory func GetProfilesPath(clusterName string) string { - return filepath.Join(GetSystemPath(clusterName), ManifestFileName) + return filepath.Join(GetSystemPath(clusterName), ProfilesManifestFileName) } // Git is an interface for basic Git operations on a single branch of a diff --git a/pkg/models/manifest.go b/pkg/models/manifest.go index 9ab5b7719b9..88b1a8ea89d 100644 --- a/pkg/models/manifest.go +++ b/pkg/models/manifest.go @@ -42,6 +42,7 @@ const ( SystemKustomizationPath = "kustomization.yaml" WegoAppPath = "wego-app.yaml" WegoConfigPath = "wego-config.yaml" + WegoProfilePath = "profiles.yaml" WegoConfigMapName = "weave-gitops-config" ) @@ -127,7 +128,7 @@ func GitopsManifests(ctx context.Context, fluxClient flux.Flux, gitProvider gitp return nil, err } - systemKustomization := CreateKustomization(clusterName, namespace, RuntimePath, SourcePath, SystemKustResourcePath, UserKustResourcePath, WegoAppPath) + systemKustomization := CreateKustomization(clusterName, namespace, RuntimePath, SourcePath, SystemKustResourcePath, UserKustResourcePath, WegoAppPath, WegoProfilePath) systemKustomizationManifest, err := yaml.Marshal(systemKustomization) if err != nil { diff --git a/pkg/models/manifest_test.go b/pkg/models/manifest_test.go index f5fd0ecdbe2..1d444681810 100644 --- a/pkg/models/manifest_test.go +++ b/pkg/models/manifest_test.go @@ -191,7 +191,7 @@ var _ = Describe("Installer", func() { fakeGitProvider.GetDefaultBranchReturns("main", nil) - systemKustomization := CreateKustomization(clusterName, testNamespace, RuntimePath, SourcePath, SystemKustResourcePath, UserKustResourcePath, WegoAppPath) + systemKustomization := CreateKustomization(clusterName, testNamespace, RuntimePath, SourcePath, SystemKustResourcePath, UserKustResourcePath, WegoAppPath, WegoProfilePath) systemKustomizationManifest, err := yaml.Marshal(systemKustomization) Expect(err).ShouldNot(HaveOccurred()) diff --git a/pkg/services/install/install_test.go b/pkg/services/install/install_test.go index 69475bb8fc7..8b82256c653 100644 --- a/pkg/services/install/install_test.go +++ b/pkg/services/install/install_test.go @@ -199,7 +199,7 @@ var _ = Describe("Installer", func() { wegoConfigManifest, err = yaml.Marshal(gitopsConfigMap) Expect(err).ShouldNot(HaveOccurred()) - systemKustomization := models.CreateKustomization(clusterName, testNamespace, models.RuntimePath, models.SourcePath, models.SystemKustResourcePath, models.UserKustResourcePath, models.WegoAppPath) + systemKustomization := models.CreateKustomization(clusterName, testNamespace, models.RuntimePath, models.SourcePath, models.SystemKustResourcePath, models.UserKustResourcePath, models.WegoAppPath, models.WegoProfilePath) systemKustomizationManifest, err = yaml.Marshal(systemKustomization) Expect(err).ShouldNot(HaveOccurred()) diff --git a/pkg/services/profiles/add.go b/pkg/services/profiles/add.go index cef81c088f8..9fc9dc233ff 100644 --- a/pkg/services/profiles/add.go +++ b/pkg/services/profiles/add.go @@ -87,7 +87,7 @@ func (s *ProfilesSvc) Add(ctx context.Context, gitProvider gitproviders.GitProvi s.Logger.Actionf("Pull Request created: %s", pr.Get().WebURL) if opts.AutoMerge { - s.Logger.Actionf("auto-merge=true; merging PR number %s", pr.Get().Number) + s.Logger.Actionf("auto-merge=true; merging PR number %v", pr.Get().Number) if err := gitProvider.MergePullRequest(ctx, configRepoUrl, pr.Get().Number, gitprovider.MergeMethodMerge, AddCommitMessage); err != nil { return err } diff --git a/pkg/services/profiles/add_test.go b/pkg/services/profiles/add_test.go index 516f7ded742..71b2598d0f6 100644 --- a/pkg/services/profiles/add_test.go +++ b/pkg/services/profiles/add_test.go @@ -65,13 +65,16 @@ var _ = Describe("Add", func() { }) When("auto-merge is enabled", func() { - It("merges the PR that was created", func() { + JustBeforeEach(func() { gitProviders.RepositoryExistsReturns(true, nil) gitProviders.GetDefaultBranchReturns("main", nil) gitProviders.GetRepoFilesReturns(makeTestFiles(), nil) clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { return true, newFakeResponseWrapper(getProfilesResp), nil }) + }) + + It("merges the PR that was created", func() { fakePR.GetReturns(gitprovider.PullRequestInfo{ WebURL: "url", Number: 42, @@ -85,12 +88,7 @@ var _ = Describe("Add", func() { When("the PR fails to be merged", func() { It("returns an error", func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetDefaultBranchReturns("main", nil) - gitProviders.GetRepoFilesReturns(makeTestFiles(), nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil - }) + fakePR.GetReturns(gitprovider.PullRequestInfo{ WebURL: "url", }) @@ -156,13 +154,6 @@ var _ = Describe("Add", func() { }) }) - It("fails if the given version was not found", func() { - addOptions.Version = "7.0.0" - err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).NotTo(BeNil()) - Expect(err).To(MatchError("version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) - }) - It("creates a helm release with that version", func() { addOptions.Version = "6.0.0" fakePR.GetReturns(gitprovider.PullRequestInfo{ @@ -175,20 +166,28 @@ var _ = Describe("Add", func() { err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) Expect(err).To(BeNil()) }) - }) - It("fails to create a pull request to write the helm release to the config repo", func() { - gitProviders.RepositoryExistsReturns(true, nil) - gitProviders.GetRepoFilesReturns(makeTestFiles(), nil) - clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { - return true, newFakeResponseWrapper(getProfilesResp), nil + It("fails if the given version was not found", func() { + addOptions.Version = "7.0.0" + err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) + Expect(err).NotTo(BeNil()) + Expect(err).To(MatchError("version '7.0.0' not found for profile 'podinfo' in prod/weave-system")) }) - gitProviders.CreatePullRequestReturns(nil, fmt.Errorf("err")) - err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) - Expect(err).NotTo(BeNil()) - Expect(err).To(MatchError("failed to create the pull request: err")) }) + When("it fails to create a pull request to write the helm release to the config repo", func() { + It("returns an error when ", func() { + gitProviders.RepositoryExistsReturns(true, nil) + gitProviders.GetRepoFilesReturns(makeTestFiles(), nil) + clientSet.AddProxyReactor("services", func(action testing.Action) (handled bool, ret restclient.ResponseWrapper, err error) { + return true, newFakeResponseWrapper(getProfilesResp), nil + }) + gitProviders.CreatePullRequestReturns(nil, fmt.Errorf("err")) + err := profilesSvc.Add(context.TODO(), gitProviders, addOptions) + Expect(err).NotTo(BeNil()) + Expect(err).To(MatchError("failed to create the pull request: err")) + }) + }) }) var _ = Describe("MakeManifestFile", func() { @@ -258,6 +257,7 @@ var _ = Describe("MakeManifestFile", func() { }) }) }) + It("fails if the manifest contains a resource that is not a HelmRelease", func() { content = "content" _, err := profiles.MakeManifestFile([]*gitprovider.CommitFile{{ diff --git a/pkg/vendorfakes/fakegitprovider/pull_request.go b/pkg/vendorfakes/fakegitprovider/pull_request.go new file mode 100644 index 00000000000..53b1c138a5d --- /dev/null +++ b/pkg/vendorfakes/fakegitprovider/pull_request.go @@ -0,0 +1,167 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fakegitprovider + +import ( + "sync" + + "github.com/fluxcd/go-git-providers/gitprovider" +) + +type PullRequest struct { + APIObjectStub func() interface{} + aPIObjectMutex sync.RWMutex + aPIObjectArgsForCall []struct { + } + aPIObjectReturns struct { + result1 interface{} + } + aPIObjectReturnsOnCall map[int]struct { + result1 interface{} + } + GetStub func() gitprovider.PullRequestInfo + getMutex sync.RWMutex + getArgsForCall []struct { + } + getReturns struct { + result1 gitprovider.PullRequestInfo + } + getReturnsOnCall map[int]struct { + result1 gitprovider.PullRequestInfo + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *PullRequest) APIObject() interface{} { + fake.aPIObjectMutex.Lock() + ret, specificReturn := fake.aPIObjectReturnsOnCall[len(fake.aPIObjectArgsForCall)] + fake.aPIObjectArgsForCall = append(fake.aPIObjectArgsForCall, struct { + }{}) + stub := fake.APIObjectStub + fakeReturns := fake.aPIObjectReturns + fake.recordInvocation("APIObject", []interface{}{}) + fake.aPIObjectMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *PullRequest) APIObjectCallCount() int { + fake.aPIObjectMutex.RLock() + defer fake.aPIObjectMutex.RUnlock() + return len(fake.aPIObjectArgsForCall) +} + +func (fake *PullRequest) APIObjectCalls(stub func() interface{}) { + fake.aPIObjectMutex.Lock() + defer fake.aPIObjectMutex.Unlock() + fake.APIObjectStub = stub +} + +func (fake *PullRequest) APIObjectReturns(result1 interface{}) { + fake.aPIObjectMutex.Lock() + defer fake.aPIObjectMutex.Unlock() + fake.APIObjectStub = nil + fake.aPIObjectReturns = struct { + result1 interface{} + }{result1} +} + +func (fake *PullRequest) APIObjectReturnsOnCall(i int, result1 interface{}) { + fake.aPIObjectMutex.Lock() + defer fake.aPIObjectMutex.Unlock() + fake.APIObjectStub = nil + if fake.aPIObjectReturnsOnCall == nil { + fake.aPIObjectReturnsOnCall = make(map[int]struct { + result1 interface{} + }) + } + fake.aPIObjectReturnsOnCall[i] = struct { + result1 interface{} + }{result1} +} + +func (fake *PullRequest) Get() gitprovider.PullRequestInfo { + fake.getMutex.Lock() + ret, specificReturn := fake.getReturnsOnCall[len(fake.getArgsForCall)] + fake.getArgsForCall = append(fake.getArgsForCall, struct { + }{}) + stub := fake.GetStub + fakeReturns := fake.getReturns + fake.recordInvocation("Get", []interface{}{}) + fake.getMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *PullRequest) GetCallCount() int { + fake.getMutex.RLock() + defer fake.getMutex.RUnlock() + return len(fake.getArgsForCall) +} + +func (fake *PullRequest) GetCalls(stub func() gitprovider.PullRequestInfo) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = stub +} + +func (fake *PullRequest) GetReturns(result1 gitprovider.PullRequestInfo) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = nil + fake.getReturns = struct { + result1 gitprovider.PullRequestInfo + }{result1} +} + +func (fake *PullRequest) GetReturnsOnCall(i int, result1 gitprovider.PullRequestInfo) { + fake.getMutex.Lock() + defer fake.getMutex.Unlock() + fake.GetStub = nil + if fake.getReturnsOnCall == nil { + fake.getReturnsOnCall = make(map[int]struct { + result1 gitprovider.PullRequestInfo + }) + } + fake.getReturnsOnCall[i] = struct { + result1 gitprovider.PullRequestInfo + }{result1} +} + +func (fake *PullRequest) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.aPIObjectMutex.RLock() + defer fake.aPIObjectMutex.RUnlock() + fake.getMutex.RLock() + defer fake.getMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *PullRequest) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ gitprovider.PullRequest = new(PullRequest) diff --git a/test/acceptance/test/profiles_test.go b/test/acceptance/test/profiles_test.go index e4e236390bc..0b9b224cb26 100644 --- a/test/acceptance/test/profiles_test.go +++ b/test/acceptance/test/profiles_test.go @@ -10,6 +10,10 @@ import ( "path/filepath" "time" + pb "github.com/weaveworks/weave-gitops/pkg/api/profiles" + "github.com/weaveworks/weave-gitops/pkg/gitproviders" + "github.com/weaveworks/weave-gitops/pkg/kube" + sourcev1beta1 "github.com/fluxcd/source-controller/api/v1beta1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -21,13 +25,9 @@ import ( "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/homedir" "sigs.k8s.io/controller-runtime/pkg/client" - - pb "github.com/weaveworks/weave-gitops/pkg/api/profiles" - "github.com/weaveworks/weave-gitops/pkg/gitproviders" - "github.com/weaveworks/weave-gitops/pkg/kube" ) -var _ = FDescribe("Weave GitOps Profiles API", func() { +var _ = Describe("Weave GitOps Profiles API", func() { var ( namespace = "test-namespace" clusterName string @@ -56,26 +56,23 @@ var _ = FDescribe("Weave GitOps Profiles API", func() { clientSet, kClient = buildKubernetesClients() }) - // AfterEach(func() { - // deleteRepo(tip.appRepoName, gitproviders.GitProviderGitHub, githubOrg) - //todo: delete helmrepository resource - // deleteWorkload(profileName, namespace) - // }) + AfterEach(func() { + deleteRepo(tip.appRepoName, gitproviders.GitProviderGitHub, githubOrg) + deleteWorkload(profileName, namespace) + }) - FIt("gets deployed and is accessible via the service", func() { + It("gets deployed and is accessible via the service", func() { By("Installing the Profiles API and setting up the profile helm repository") appRepoRemoteURL = "git@github.com:" + githubOrg + "/" + tip.appRepoName + ".git" installAndVerifyWego(namespace, appRepoRemoteURL) deployProfilesHelmRepository(kClient, namespace) - time.Sleep(time.Second * 60) By("Getting a list of profiles") - Eventually(func() error { + Eventually(func() int { resp, statusCode, err = kubernetesDoRequest(namespace, wegoService, wegoPort, "/v1/profiles", clientSet) - return err - }, "10s", "1s").Should(Succeed()) + return statusCode + }, "60s", "1s").Should(Equal(http.StatusOK)) Expect(err).NotTo(HaveOccurred()) - Expect(statusCode).To(Equal(http.StatusOK)) profiles := pb.GetProfilesResponse{} Expect(json.Unmarshal(resp, &profiles)).To(Succeed()) @@ -112,34 +109,20 @@ podinfo Podinfo Helm chart for Kubernetes 6.0.0,6.0.1 Expect(err).NotTo(HaveOccurred()) Expect(string(values)).To(ContainSubstring("# Default values for podinfo")) - By("Adding a profile to the cluster through the CLI") - stdOut, stdErr := runCommandAndReturnStringOutput(fmt.Sprintf("%s add profile --name %s --version %s --namespace %s --cluster %s --config-repo %s --auto-merge %v", gitopsBinaryPath, profileName, "6.0.1", namespace, clusterName, appRepoRemoteURL, true)) + By("Adding a profile to a cluster") + stdOut, stdErr := runCommandAndReturnStringOutput(fmt.Sprintf("%s add profile --name %s --version 6.0.1 --namespace %s --cluster %s --config-repo %s --auto-merge", gitopsBinaryPath, profileName, namespace, clusterName, appRepoRemoteURL)) Expect(stdErr).To(BeEmpty()) - fmt.Println(stdOut) - // args := []string{ - // "add", - // "profile", - // "--name", profileName, - // "--version", "6.0.1", - // "--cluster", clusterName, - // "--namespace", namespace, - // "--config-repo", appRepoRemoteURL, - // "--auto-merge", "true", - // } - // cmd := exec.Command("wego", args...) - // resp, err = cmd.CombinedOutput() - // fmt.Println(err) - // fmt.Println(string(resp)) - - time.Sleep(time.Second * 60) - - By("Verifying that the profile has been deployed on the cluster") - Eventually(func() error { - resp, statusCode, err = kubernetesDoRequest(namespace, profileName, "9898", "/healthz", clientSet) - return err - }, "10s", "1s").Should(Succeed()) - Expect(string(resp)).To(Equal(200)) - Expect(statusCode).To(Equal(http.StatusOK)) + Expect(stdOut).To(ContainSubstring( + `Adding profile: + +Name: podinfo +Version: 6.0.1`)) + + By("Verifying that the profile has been installed on the cluster") + Eventually(func() int { + resp, statusCode, err = kubernetesDoRequest(namespace, clusterName+"-"+profileName, "9898", "/healthz", clientSet) + return statusCode + }, "120s", "1s").Should(Equal(http.StatusOK)) }) It("profiles are installed into a different namespace", func() { diff --git a/test/acceptance/test/utils.go b/test/acceptance/test/utils.go index c93172b94a5..ae1e49197b2 100644 --- a/test/acceptance/test/utils.go +++ b/test/acceptance/test/utils.go @@ -238,18 +238,19 @@ func ResetOrCreateClusterWithName(namespace string, deleteWegoRuntime bool, clus } if provider == "kind" { + var kindCluster string if clusterName == "" { - clusterName = provider + "-" + RandString(6) + kindCluster = RandString(6) } + clusterName = provider + "-" + kindCluster - log.Infof("Creating a kind cluster %s", clusterName) + log.Infof("Creating a kind cluster %s", kindCluster) var err error - if keepExistingClusters { - err = runCommandPassThrough([]string{}, "./scripts/kind-multi-cluster.sh", clusterName, "kindest/node:v"+k8sVersion) + err = runCommandPassThrough([]string{}, "./scripts/kind-multi-cluster.sh", kindCluster, "kindest/node:v"+k8sVersion) } else { - err = runCommandPassThrough([]string{}, "./scripts/kind-cluster.sh", clusterName, "kindest/node:v"+k8sVersion) + err = runCommandPassThrough([]string{}, "./scripts/kind-cluster.sh", kindCluster, "kindest/node:v"+k8sVersion) } if err != nil {