-
Notifications
You must be signed in to change notification settings - Fork 142
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
Add namespace to wego-app manifests #1041
Changes from 6 commits
849b2d5
a344f73
36c92d4
414de86
9a4488c
d72be21
8a8604a
ff228ab
092987f
83104ba
471930c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,68 +2,73 @@ package manifests | |
|
||
import ( | ||
"bytes" | ||
"embed" | ||
_ "embed" | ||
"fmt" | ||
"io/fs" | ||
"path/filepath" | ||
"text/template" | ||
) | ||
|
||
"github.com/pkg/errors" | ||
const ( | ||
wegoManifestsDir = "wego-app" | ||
) | ||
|
||
var Manifests [][]byte | ||
var ( | ||
Manifests [][]byte | ||
//go:embed crds/wego.weave.works_apps.yaml | ||
AppCRD []byte | ||
//go:embed wego-app/* | ||
wegoAppTemplates embed.FS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Cleans things up a lot 👍 |
||
) | ||
|
||
//go:embed crds/wego.weave.works_apps.yaml | ||
var AppCRD []byte | ||
type WegoAppParams struct { | ||
Version string | ||
Namespace string | ||
} | ||
|
||
//go:embed wego-app/deployment.yaml | ||
var WegoAppDeployment []byte | ||
// GenerateWegoManifests generates wego-app manifest from a template | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: manifest => manifests |
||
func GenerateWegoAppManifests(params WegoAppParams) ([][]byte, error) { | ||
manifests := [][]byte{} | ||
|
||
type deploymentParameters struct { | ||
Version string | ||
} | ||
templates, _ := fs.ReadDir(wegoAppTemplates, wegoManifestsDir) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we handle error here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's very unlikely this will ever return any error because it's reading directly from memory, but it doesn't hurt adding the err check |
||
for _, template := range templates { | ||
tplName := template.Name() | ||
|
||
var errInjectingValuesToTemplate = errors.New("error injecting values to template") | ||
data, err := fs.ReadFile(wegoAppTemplates, filepath.Join(wegoManifestsDir, tplName)) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed reading template %s: %w", tplName, err) | ||
} | ||
|
||
// GenerateWegoAppDeploymentManifest generates wego-app deployment manifest from a template | ||
func GenerateWegoAppDeploymentManifest(version string) ([]byte, error) { | ||
deploymentValues := deploymentParameters{Version: version} | ||
manifest, err := executeTemplate(tplName, string(data), params) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed executing template: %s: %w", tplName, err) | ||
} | ||
|
||
template := template.New("DeploymentTemplate") | ||
manifests = append(manifests, manifest) | ||
} | ||
|
||
var err error | ||
return manifests, nil | ||
} | ||
|
||
template, err = template.Parse(string(WegoAppDeployment)) | ||
func executeTemplate(name string, tplData string, params WegoAppParams) ([]byte, error) { | ||
template, err := template.New(name).Parse(tplData) | ||
if err != nil { | ||
return nil, fmt.Errorf("error parsing template %w", err) | ||
return nil, fmt.Errorf("error parsing template %s: %w", name, err) | ||
} | ||
|
||
deploymentYaml := &bytes.Buffer{} | ||
yaml := &bytes.Buffer{} | ||
|
||
err = template.Execute(deploymentYaml, deploymentValues) | ||
err = template.Execute(yaml, params) | ||
if err != nil { | ||
return nil, fmt.Errorf("%s %w", errInjectingValuesToTemplate, err) | ||
return nil, fmt.Errorf("error injecting values to template: %w", err) | ||
} | ||
|
||
return deploymentYaml.Bytes(), nil | ||
return yaml.Bytes(), nil | ||
} | ||
|
||
//go:embed wego-app/service-account.yaml | ||
var WegoAppServiceAccount []byte | ||
|
||
//go:embed wego-app/service.yaml | ||
var WegoAppService []byte | ||
|
||
//go:embed wego-app/role.yaml | ||
var WegoAppRole []byte | ||
|
||
//go:embed wego-app/role-binding.yaml | ||
var WegoAppRoleBinding []byte | ||
|
||
func init() { | ||
Manifests = [][]byte{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we think this will ever have more than one manifest in it any more? If not, maybe this should just be an "AppCRDManifest" variable 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, I'll remove it |
||
AppCRD, | ||
WegoAppServiceAccount, | ||
WegoAppRoleBinding, | ||
WegoAppRole, | ||
WegoAppService, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,23 @@ | ||
package manifests | ||
|
||
import ( | ||
"strings" | ||
|
||
. "github.com/onsi/ginkgo" | ||
. "github.com/onsi/gomega" | ||
"github.com/weaveworks/weave-gitops/cmd/gitops/version" | ||
appsv1 "k8s.io/api/apps/v1" | ||
"sigs.k8s.io/yaml" | ||
) | ||
|
||
var _ = Describe("Testing WegoAppDeployment", func() { | ||
It("should contain the right version", func() { | ||
v := version.Version | ||
deploymentYaml, err := GenerateWegoAppDeploymentManifest(v) | ||
Expect(err).NotTo(HaveOccurred()) | ||
|
||
var Deployment appsv1.Deployment | ||
err = yaml.Unmarshal(deploymentYaml, &Deployment) | ||
manifests, err := GenerateWegoAppManifests(WegoAppParams{Version: version.Version, Namespace: "my-namespace"}) | ||
Expect(err).NotTo(HaveOccurred()) | ||
|
||
Expect(Deployment.Spec.Template.Spec.Containers[0].Image).To(ContainSubstring(v)) | ||
for _, m := range manifests { | ||
if strings.Contains(string(m), "kind: Deployment") { | ||
Expect(string(manifests[0])).To(ContainSubstring("namespace: my-namespace")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line and the next: manifests[0] => m |
||
Expect(string(manifests[0])).To(ContainSubstring(version.Version)) | ||
} | ||
} | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ var _ = Describe("Sync", func() { | |
Expect(err.Error()).To(HavePrefix("failed getting application")) | ||
}) | ||
|
||
It("sets proper annotation tag to the resource", func() { | ||
XIt("sets proper annotation tag to the resource", func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a temporary fix until the sync test is working? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||
ready := make(chan bool) | ||
|
||
go func() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package gitops | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"os" | ||
|
@@ -62,7 +63,6 @@ func (g *Gitops) Install(params InstallParams) (map[string][]byte, error) { | |
if params.DryRun { | ||
return systemManifests, nil | ||
} else { | ||
|
||
for _, manifest := range manifests.Manifests { | ||
if err := g.kube.Apply(ctx, manifest, params.Namespace); err != nil { | ||
return nil, fmt.Errorf("could not apply manifest: %w", err) | ||
|
@@ -74,15 +74,17 @@ func (g *Gitops) Install(params InstallParams) (map[string][]byte, error) { | |
version = "latest" | ||
} | ||
|
||
wegoAppDeploymentManifest, err := manifests.GenerateWegoAppDeploymentManifest(version) | ||
wegoAppManifests, err := manifests.GenerateWegoAppManifests(manifests.WegoAppParams{Version: version, Namespace: params.Namespace}) | ||
if err != nil { | ||
return nil, fmt.Errorf("error generating wego-app deployment, %w", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we update this error message to match the new behavior? |
||
} | ||
|
||
systemManifests["wego-app.yaml"] = wegoAppDeploymentManifest | ||
if err := g.kube.Apply(ctx, wegoAppDeploymentManifest, params.Namespace); err != nil { | ||
return nil, fmt.Errorf("could not apply wego-app deployment manifest: %w", err) | ||
for _, m := range wegoAppManifests { | ||
if err := g.kube.Apply(ctx, m, params.Namespace); err != nil { | ||
return nil, fmt.Errorf("error applying wego-app manifest %s: %w", string(m), err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: "string" not needed here |
||
} | ||
} | ||
|
||
systemManifests["wego-app.yaml"] = bytes.Join(wegoAppManifests, []byte("---\n")) | ||
} | ||
|
||
return systemManifests, nil | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ package acceptance | |
import ( | ||
"context" | ||
"fmt" | ||
"os/exec" | ||
|
||
wego "github.com/weaveworks/weave-gitops/api/v1alpha1" | ||
"github.com/weaveworks/weave-gitops/manifests" | ||
|
@@ -191,4 +192,24 @@ var _ = Describe("Weave GitOps Install Tests", func() { | |
Eventually(errOutput).Should(ContainSubstring(`Error from server (NotFound): namespaces "` + WEGO_DEFAULT_NAMESPACE + `" not found`)) | ||
}) | ||
}) | ||
|
||
It("Verify wego app is deployed", func() { | ||
namespace := "wego-system" | ||
|
||
By("And I have a brand new cluster", func() { | ||
_, err := ResetOrCreateCluster(namespace, true) | ||
Expect(err).ShouldNot(HaveOccurred()) | ||
}) | ||
|
||
installAndVerifyWego(namespace) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will need a URL with the new directory structure changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you mean? |
||
|
||
By("And the wego-app is up and running", func() { | ||
command := exec.Command("sh", "-c", fmt.Sprintf("kubectl wait --for=condition=Ready --timeout=60s -n %s --all pods --selector='app=wego-app'", namespace)) | ||
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) | ||
Expect(err).ShouldNot(HaveOccurred()) | ||
Eventually(session, INSTALL_PODS_READY_TIMEOUT).Should(gexec.Exit()) | ||
}) | ||
|
||
_ = waitForNamespaceToTerminate(namespace, NAMESPACE_TERMINATE_TIMEOUT) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we can remove this array now? It is being filled out in init but only with the AppCRD which is already public, so then we can access AppCRD directly instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep