Add Flux support to install resources from git source: #255

matrus2 · 2023-05-10T11:40:09Z

Hello,

This a proposal and MVP for integration e2e framework with Flux. This one include:

create and delete GitRepository,
create and delete Kustmozation,
example of how to use the functionality.

Please comment & review. Once first part is done we can focus to provide all other functionalities from Flux e.g. Helm repository reconciliation support.

k8s-ci-robot · 2023-05-10T11:40:17Z

Hi @matrus2. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ShwethaKumbla

@matrus2 Thanks for this great contribution.

ShwethaKumbla · 2023-05-11T06:29:38Z

examples/third_party_integration/flux/flux_test.go

@@ -0,0 +1,86 @@
+/*
+Copyright 2021 The Kubernetes Authors.


ShwethaKumbla · 2023-05-11T06:40:28Z

pkg/envfuncs/third_party_funcs.go

@@ -0,0 +1,40 @@
+/*


This file can be moved under third-party/flux to maintain the structuring we followed for the other third-party support.

I second that. Putting flux references in pkg would make it a required dependency when pulling the project, whether you want it or not.

ShwethaKumbla · 2023-05-11T06:45:28Z

/ok-to-test

ShwethaKumbla · 2023-05-11T06:45:50Z

/cc @vladimirvivien

vladimirvivien

This is a great contribution. And it is a great start.
Let's continue the review process to get it where it needs to land.

vladimirvivien · 2023-05-12T12:31:28Z

examples/third_party_integration/flux/flux_test.go

+	kustomizationName := "hello-world"
+
+	feature := features.New("Install resources by flux").
+		Setup(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {


Suggestion: this setup/teardown probably belongs in a test environment setup/finish block in TestMain.

That is a great point, agree. It should go to setup block.

vladimirvivien · 2023-05-12T12:34:01Z

pkg/envfuncs/third_party_funcs.go

@@ -0,0 +1,40 @@
+/*


I second that. Putting flux references in pkg would make it a required dependency when pulling the project, whether you want it or not.

vladimirvivien · 2023-05-12T12:43:10Z

examples/third_party_integration/flux/flux_test.go

+				t.Fatal("failed to create git repository", err)
+			}
+			// Set --prune so that once deleteKustomization will be called all corresponding resources will be removed
+			err = manager.CreateKustomization(kustomizationName, "GitRepository/hello-world.flux-system", flux.WithPath("template"), flux.WithArgs("--target-namespace", c.Namespace(), "--prune"))


Question: where is the kustomization file GitRepository/hello-world.flux-system coming from ? Is it pulled from the repo?

GitRepository/hello-world.flux-system is a reference to GitRepository object in the flux-system namespace created in previous step. Basically, GitRepository is the declaration of a git repository and reference to the source of manifest files:

apiVersion: source.toolkit.fluxcd.io/v1 kind: GitRepository metadata: name: hello-world namespace: flux-system spec: interval: 10m ref: branch: main url: ssh://[email protected]/matrus2/go-hello-world

The Kustomization is a specification, which files form Git Repository and how they should be deployed. In this example we deploy files from path template within 1 minute interval to target namespace default. See below the example:

apiVersion: kustomize.toolkit.fluxcd.io/v1 kind: Kustomization metadata: name: hello-world namespace: flux-system spec: interval: 1m targetNamespace: default path: template prune: true sourceRef: kind: GitRepository name: hello-world

vladimirvivien

This is awesome @matrus2 .
I would suggest to add documentation comments for the functions in package third_party/flux

harshanarayana · 2023-06-12T12:42:58Z

examples/third_party_integration/flux/main_test.go

+		envfuncs.CreateKindCluster(kindClusterName),
+		envfuncs.CreateNamespace(namespace),
+		flux.InstallFlux(),
+		flux.CreateGitRepo(gitRepoName, "https://github.com/matrus2/go-hello-world", flux.WithBranch("main")),


I think we need to find a better way to manage this repository may be. I would rather have this repo co-located with some kubernetes-sigs org than have it in a personal repo and then use it here. cc @vladimirvivien

As an alternative, We could actually have a self-referencing repo and point the test to use e2e-framework repo itself. We can create the sample customization template and other bits and use that from here directly ?

Yes, in fact this was the plan to move it here to e2e-framework repo after merge of this PR. I see two possible scenarios:

Merge this PR and later move the manifests to e2e-framework and at the same time harden the security and reference not a branch, but a commit SHA, so that we ensure no new unexpected changes will land to external repo.

Split this PR and first move manifests from external repo to e2e-framework, and in the next PR reference it via Flux.

Please advice which one you prefer.

harshanarayana · 2023-06-12T12:44:02Z

third_party/flux/flux_setup.go

+
+const NoFluxInstallationFoundMsg = "flux needs to be installed within a cluster first"
+
+func InstallFlux() env.Func {


Do you think it would be a good idea to take the namespace as an optional argument here so that one can easily install this on a non standard flux-system namespace ?

harshanarayana · 2023-06-12T12:46:20Z

third_party/flux/cmd_manager.go

+const (
+	Git Source = iota
+	Bucket
+	Helm
+	Oci
+)


Would it not be easier to define a custom type for Source as string and define these as string values directly instead of using .String() function ?

harshanarayana · 2023-06-12T12:52:55Z

third_party/flux/flux_setup.go

+	}
+}
+
+func UninstallFlux() env.Func {


Please add some documentation for these helper functions.

harshanarayana · 2023-06-12T13:01:01Z

examples/third_party_integration/flux/main_test.go

+		flux.InstallFlux(),
+		flux.CreateGitRepo(gitRepoName, "https://github.com/matrus2/go-hello-world", flux.WithBranch("main")),
+		flux.CreateKustomization(ksName, "GitRepository/"+gitRepoName+".flux-system", flux.WithPath("template"), flux.WithArgs("--target-namespace", namespace, "--prune")),
+	)


I also noticed that the repo https://github.com/matrus2/go-hello-world/blob/main/template/deployment.yaml#L18 hardcodes the image reference. Don't we need to create a ImageUpdateAutomation for it to really do the right thing ? Also I am not sure if we have any precedence for this where the image from a personal dockerhub repo is included into the rest util. May be we should move all of these repo and images to a common location and manage them like we do for rest of the test images and others in k8s ?

For the purpose of this test the image reference is hardcoded and this is one of the use case. For example you may have some prerequisite components to install inside the cluster like CNI, before deploy your staff. The example here is the simplest possible.
For moving the manifests see my answer above, for image reference not sure if you have any kuberentes-sigs registry.

matrus2 · 2023-06-19T11:28:08Z

/retest

matrus2 · 2023-06-22T07:30:02Z

@vladimirvivien @harshanarayana Please take a look in spare time. I think I answered to all your comments. I would like to continue with Flux and add Helm support, which gives a lot of more flexibility and add possibility to write more complex use cases as examples.

harshanarayana · 2023-06-22T11:38:15Z

@matrus2 Sorry, this one totally went missing from my radar.

Thanks for taking care of this and working towards adding this awesome feature into the framework. With regards to the sample app part of the questions, I think we might be able to do it in two phases like you were suggesting above.

Add the sample/example hello world project to this repo as the first PR
Reference your current PR to use the main branch from e2e-framework repo itself after that changes are merged

This way, we can keep all the test data co-located and can easily make changes to new things. I wish there was a way to setup flux to use a local file system path as a repo instead of an actual URL. That would have simplified a whole lot of this work! I would prefer to hear from @vladimirvivien @cpanato @ShwethaKumbla on what their view on this is before we can make any further decision.

I will go over the rest of the changes and if I find something needing a change, I will leave a comment. Give me until Weekend to sort a few things out.

matrus2 · 2023-06-23T14:02:27Z

@harshanarayana Thanks for taking time and review this one. Template migrated in #283, reference updated.

vladimirvivien · 2023-07-01T13:21:39Z

@matrus2 This will be great. Always a good thing when this framework can be integrated with other projects.

@harshanarayana thanks for looking at this so thoroughly🎉

vladimirvivien · 2023-07-01T13:21:53Z

/retest

matrus2 · 2023-07-01T13:49:58Z

@vladimirvivien Thanks for taking a look at this. The test won't pass untill #283 is merged as files reference template directory.

matrus2 · 2023-07-10T07:36:53Z

/retest

* Include create and delete GitRepository * Include create and delete Kustmozation * Provide basic example

harshanarayana · 2023-07-12T06:11:34Z

third_party/flux/cmd_manager.go

+}
+
+func (m *Manager) run(opts *Opts) (err error) {
+	if m.e.Prog().Avail("flux") == "" {


cc @matrus2 Now that we have the template merged, this is more or less ready to be merged as well.

Recently we enabled a way to use a custom path for these binaries that we use in the framework. i.e You can specify a path outside of the $PATH that you have your binaries in. Can we have a mechanism similar to that here? We have the same behavior on kind/and helm already. A way to customize teh path using WithPath kind of helper would be great to have.

This is now fixed.

harshanarayana · 2023-07-12T15:36:08Z

@vladimirvivien Would you mind taking a look at this and see if you have any more pending request that needs to be addressed on this ?

matrus2 · 2023-07-26T11:01:20Z

Hi @vladimirvivien, any chance to wrap this up?

vladimirvivien

Hi @matrus2, apologies for the delay, I was on vacation.
Thanks for hanging in there and doing this valuable contribution.
Thanks @harshanarayana for the thorough reviews!!

k8s-ci-robot · 2023-08-01T11:56:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matrus2, vladimirvivien

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [vladimirvivien]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels May 10, 2023

k8s-ci-robot requested review from cpanato and ShwethaKumbla May 10, 2023 11:40

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 10, 2023

ShwethaKumbla reviewed May 11, 2023

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 11, 2023

k8s-ci-robot requested a review from vladimirvivien May 11, 2023 06:45

vladimirvivien reviewed May 12, 2023

View reviewed changes

vladimirvivien reviewed Jun 10, 2023

View reviewed changes

harshanarayana reviewed Jun 12, 2023

View reviewed changes

matrus2 mentioned this pull request Jun 23, 2023

Add templates for flux example #283

Merged

matrus2 added 3 commits July 10, 2023 16:41

Add Flux support to install resources from git source:

2aebadc

* Include create and delete GitRepository * Include create and delete Kustmozation * Provide basic example

Restructure flux integration

31ed661

Add permissions to flux install

07c8302

matrus2 added 4 commits July 10, 2023 16:41

Fix typo in install flux script

9ec6366

Make installation to work

60ea14d

Add documentation

88a9d1c

Update reference to e2e-framework

607e2ea

Fix lint errors

2ddff14

matrus2 force-pushed the flux branch from 061b223 to 2ddff14 Compare July 10, 2023 14:54

Remove obsolete comment from flux example

efd73eb

harshanarayana reviewed Jul 12, 2023

View reviewed changes

Add possibility to specify flux path

aab80f0

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 12, 2023

matrus2 requested a review from vladimirvivien July 26, 2023 11:01

vladimirvivien approved these changes Aug 1, 2023

View reviewed changes

k8s-ci-robot assigned vladimirvivien Aug 1, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2023

k8s-ci-robot merged commit 1af0fd6 into kubernetes-sigs:main Aug 1, 2023

harshanarayana mentioned this pull request Aug 14, 2023

Release Ticket: v0.3.0 of e2e-framework #307

Closed

matrus2 mentioned this pull request Oct 27, 2023

REQUEST: New membership for matrus2 kubernetes/org#4553

Closed

9 tasks


		const NoFluxInstallationFoundMsg = "flux needs to be installed within a cluster first"

		func InstallFlux() env.Func {

Add Flux support to install resources from git source: #255

Add Flux support to install resources from git source: #255

Conversation

matrus2 commented May 10, 2023

k8s-ci-robot commented May 10, 2023

ShwethaKumbla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShwethaKumbla commented May 11, 2023

ShwethaKumbla commented May 11, 2023

vladimirvivien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladimirvivien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matrus2 commented Jun 19, 2023

matrus2 commented Jun 22, 2023

harshanarayana commented Jun 22, 2023

matrus2 commented Jun 23, 2023

vladimirvivien commented Jul 1, 2023

vladimirvivien commented Jul 1, 2023

matrus2 commented Jul 1, 2023

matrus2 commented Jul 10, 2023

harshanarayana Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshanarayana commented Jul 12, 2023

matrus2 commented Jul 26, 2023

vladimirvivien left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 1, 2023

harshanarayana Jul 12, 2023 •

edited

Loading