provider: added k3d provider and node lifecycle handlers #441

harshanarayana · 2024-07-13T14:35:25Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR enables a new E2EClusterProvider for k3d based infra.

This change includes the following changes and features.

Added a new Interface type E2EClusterProviderWithImageLoaderAndNodeLifecycle which can be used to setup providers that extend the cluster lifecycle function around the nodes.
Enabled a k3d based provider with support for Node lifecycle management.
Existing Image loader related function and interfaces were augmented with args ...string to be able to provide additional arguments in case if the image load handlers need some of the additional config.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

I have also extended the E2EClusterProviderWithImageLoader 's Image load related methods to take an additional arg ...string value to account for additional options that one can pass while performing image load operation alone.

This comes in handy with k3d where it provides a few different mode of loading images and cleanup workflows.

This change also doesn't break any existing API contract extended by the envfuncs.

Does this PR introduce a user-facing change?

enabled a new `E2eClusterProvider` for `k3d` that can be used as a new cluster provider for running e2e tests.

Additional documentation e.g., Usage docs, etc.:

NA

harshanarayana · 2024-07-13T14:41:35Z

/cc @cpanato

harshanarayana · 2024-07-14T16:10:56Z

go install of k3d is failing due to an issue similar to k3d-io/k3d#1324

harshanarayana · 2024-07-14T16:30:43Z

/hold

Holding this under the k3d install issue is resolved.

harshanarayana · 2024-07-16T05:51:03Z

support/utils/command.go

+	p := commandRunner.NewProc(installCommand)
+	p.SetStdout(&stdout)
+	p.SetStderr(&stderr)
+	result := p.Run()
+	if result.Err() != nil {
+		return "", fmt.Errorf("failed to install %s: %s: \n %s", pPath, result.Result(), stderr.String())


Adding these to be able to capture and log the install failures better during the provider install.

harshanarayana · 2024-07-16T05:53:52Z

/unhold

cpanato

/lgtm
thanks

k8s-ci-robot · 2024-07-16T11:27:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, harshanarayana

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~~ [cpanato,harshanarayana]

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

harshanarayana · 2024-07-16T11:28:53Z

/hold

harshanarayana · 2024-07-16T11:29:32Z

Holding this until @cpanato @vladimirvivien @ShwethaKumbla also takes look

harshanarayana · 2024-07-16T12:04:10Z

/retest

cpanato

/lgtm

vladimirvivien

@harshanarayana this a great (massive) PR.
Thanks for doing this and I am sure the community will appreciate the k3d integration.
I left some comments and we can work on getting this to the finish line.

examples/k3d/main_test.go

pkg/stepfuncs/nodelifecycle_funcs.go

support/k3d/k3d.go

support/kind/kind.go

vladimirvivien · 2024-07-20T15:02:50Z

support/types.go

+// test out how the Remove operation impacts your workflow.
+// Or you want to simulate a case where one or more node of your cluster is down and you want to see how
+// your application reacts to such failure events.
+type E2EClusterProviderWithImageLoaderAndNodeLifecycle interface {


Oof, that is a long name.
Let's reconsider making it simpler or break into multiple (embedded) interfaces.

E2EClusterProvier

E2EClusterProviderWithImgLoader
*E2EClusterProviderWithLifeCycle

Ack. using E2EClusterProviderWithLifeCycle instead

@vladimirvivien Do you want to check the image loader related interface name too ?

k8s-ci-robot · 2024-07-22T05:19:47Z

New changes are detected. LGTM label has been removed.

harshanarayana · 2024-07-22T05:19:57Z

@vladimirvivien PTAL. I has pushed the requested changes.

ShwethaKumbla

LGTM

vladimirvivien

Making progress @harshanarayana
Added some more comments.

vladimirvivien · 2024-08-01T13:14:55Z

support/k3d/k3d.go

+	}
+	log.V(4).InfoS("Stopping node in k3d cluster", "command", cmd)
+	p, stdout, stderr := utils.FetchSeperatedCommandOutput(cmd)
+	if p.Err() != nil || (p.Exited() && p.ExitCode() != 0) {


Oh boy, you are pushing that package (gexe) to it's limit. Love it 😉

vladimirvivien · 2024-08-01T13:17:06Z

pkg/utils/nodelifecycle.go

+
+// PerformNodeLifecycleOperation performs a node operation on a cluster. These operations can range from Add/Remove/Start/Stop.
+// This helper is re-used in both node lifecycle handler used as types.StepFunc or env.Func
+func PerformNodeLifecycleOperation(ctx context.Context, action support.NodeOperation, node *support.Node, args ...string) error {


Does this function only applies when using K3d or do you think orther providers will use that.
If only k3d, maybe it belongs in third_party package path if that is the case.

Other providers like minikube can support this.. And kwok technically can be simulated to enable node add and remove workflows. So it can work with other providers as well. We can do that even for kind. But we will have to use adhoc docker command to simulate power on and off kind of workflows

support/k3d/k3d.go

This change includes the following changes and features. 1. Added a new Interface type `E2EClusterProviderWithLifeCycle` which can be used to setup providers that extend the cluster lifecycle function around the nodes. 2. Enabled a `k3d` based provider with support for Node lifecycle management. 3. Existing Image loader related function and interfaces were augmented with `args ...string` to be able to provide additional arguments in case if the image load handlers need some of the additional config.

harshanarayana · 2024-08-15T13:39:08Z

@vladimirvivien PTAL when you can. Addressed the required changes in the message for wait for control plane.

vladimirvivien

More comments.

vladimirvivien · 2024-08-27T13:41:01Z

support/k3d/k3d.go

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


This should be in third-party.
I see that kwok in that package, but it should have also been in third-party.
package support is a place for API definitions for third-party support.
kind was supposed to be an example implementation for others (maybe it should have been in third-party as well)

Good point.. I can move them all. But we have to retain some sort of aliasing to aid the import backward compatibility.. At least for kwok and the kind that we support today in an available release.. Right?

If you are ok with that, I will take care of the required package refactoring

Hi @harshanarayana if type aliasing will help maintain backward compat, then do it.
At the very least, we should just move kwok. Thanks for doing it.

vladimirvivien · 2024-08-27T13:45:16Z

support/utils/command.go

-	p = commandRunner.RunProc("ls $GOPATH/bin")
-	if p.Err() != nil {
-		return "", fmt.Errorf("failed to install %s: %s", pPath, p.Err())
+	p = commandRunner.NewProc("ls $GOPATH/bin")


Do you think gexe could be improved to make output caputure easier ? If so, please open an issue.

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jul 13, 2024

k8s-ci-robot requested review from ShwethaKumbla and vladimirvivien July 13, 2024 14:35

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 13, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch 2 times, most recently from 0c3a6e3 to d25b6ab Compare July 13, 2024 14:38

k8s-ci-robot requested a review from cpanato July 13, 2024 14:41

harshanarayana mentioned this pull request Jul 14, 2024

[BUG] Cannot install k3d v5.5.1 using Go 1.22.x due to replace directive k3d-io/k3d#1468

Closed

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch 2 times, most recently from 3b76b81 to a70ae18 Compare July 16, 2024 05:50

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 16, 2024

harshanarayana commented Jul 16, 2024

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch from a70ae18 to afe220a Compare July 16, 2024 11:21

harshanarayana changed the title ~~provider: enable k3d based cluster provider~~ provider: added k3d provider and node lifecycle handlers Jul 16, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch 2 times, most recently from 46696b8 to 6a755b5 Compare July 16, 2024 11:24

cpanato approved these changes Jul 16, 2024

View reviewed changes

k8s-ci-robot assigned cpanato Jul 16, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 16, 2024

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch from 6a755b5 to 4c0f410 Compare July 16, 2024 15:51

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 16, 2024

cpanato reviewed Jul 16, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 16, 2024

vladimirvivien reviewed Jul 20, 2024

View reviewed changes

harshanarayana force-pushed the feature/k3d-provider-support branch from 4c0f410 to 3a6934c Compare July 22, 2024 05:19

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 22, 2024

harshanarayana force-pushed the feature/k3d-provider-support branch from 3a6934c to 1597cf5 Compare July 22, 2024 05:25

ShwethaKumbla reviewed Jul 27, 2024

View reviewed changes

vladimirvivien reviewed Aug 1, 2024

View reviewed changes

harshanarayana force-pushed the feature/k3d-provider-support branch from 1597cf5 to 3d1be3f Compare August 15, 2024 13:38

vladimirvivien reviewed Aug 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider: added k3d provider and node lifecycle handlers #441

provider: added k3d provider and node lifecycle handlers #441

harshanarayana commented Jul 13, 2024 •

edited

Loading

harshanarayana commented Jul 13, 2024

harshanarayana commented Jul 14, 2024

harshanarayana commented Jul 14, 2024

harshanarayana Jul 16, 2024

harshanarayana commented Jul 16, 2024

cpanato left a comment

k8s-ci-robot commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

cpanato left a comment

vladimirvivien left a comment

vladimirvivien Jul 20, 2024

harshanarayana Jul 22, 2024

harshanarayana Jul 22, 2024

k8s-ci-robot commented Jul 22, 2024

harshanarayana commented Jul 22, 2024

ShwethaKumbla left a comment

vladimirvivien left a comment

vladimirvivien Aug 1, 2024

vladimirvivien Aug 1, 2024

harshanarayana Aug 1, 2024 •

edited

Loading

harshanarayana commented Aug 15, 2024

vladimirvivien left a comment

vladimirvivien Aug 27, 2024

harshanarayana Aug 28, 2024

vladimirvivien Aug 31, 2024

vladimirvivien Aug 27, 2024

provider: added k3d provider and node lifecycle handlers #441

Are you sure you want to change the base?

provider: added k3d provider and node lifecycle handlers #441

Conversation

harshanarayana commented Jul 13, 2024 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., Usage docs, etc.:

harshanarayana commented Jul 13, 2024

harshanarayana commented Jul 14, 2024

harshanarayana commented Jul 14, 2024

Choose a reason for hiding this comment

harshanarayana commented Jul 16, 2024

cpanato left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

harshanarayana commented Jul 16, 2024

cpanato left a 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

k8s-ci-robot commented Jul 22, 2024

harshanarayana commented Jul 22, 2024

ShwethaKumbla left a 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

harshanarayana Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

harshanarayana commented Aug 15, 2024

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

harshanarayana commented Jul 13, 2024 •

edited

Loading

harshanarayana Aug 1, 2024 •

edited

Loading