Add `clustersim` command for manual MCM testing by takoverflow · Pull Request #1152 · gardener/machine-controller-manager

takoverflow · 2026-09-11T13:25:48Z

What this PR does / why we need it:
Introduces standalone clustersim tool to create virtual clusters with workload consisting of MachineClasses and MachineDeployments to aid in manual testing. Please check its README.md for more details.

Which issue(s) this PR fixes:
Fixes part of #1120

Special notes for your reviewer:
The line diff is bloated because of the instances.json file, which consists of capacity information for various AWS instances. If deemed overkill, it can be replaced by a list of top 20 frequently used instances for all providers.

/invite @elankath

Release note:

Added a command line program `clustersim` to aid in manual MCM testing via virtual clusters

Signed-off-by: Prashant Tak <prashant.tak@sap.com>

gardener-prow · 2026-09-11T13:25:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ashwani2k for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

Signed-off-by: Prashant Tak <prashant.tak@sap.com>

aaronfern

Thanks for the PR!
I have a few questions and nits

aaronfern · 2026-09-30T05:21:52Z

 go 1.26.2

 require (
+	github.com/gardener/gardener/pkg/apis v1.137.0


This is quite outdated. Do you want to use a later version?

It doesn't make a difference from clustersim PoV, since we only rely on the typed data for ViewerKubeconfig and Shoot resources, and the information that we capture is very minimal from the latter (Just the seedname tbh).

I'll still bump up the version for now.

aaronfern · 2026-09-30T05:59:23Z

+
+### `build`
+
+Builds the specified components, fallsback to building both `machine-controller-manager` and the `simulated-provider` if unspecifed. Requires a mandatory flag `source-dir` pointing to the `machine-controller-manager` source root directory.


It would be nice if source-dir had a default of ../../../../ and users can override if they intend on using a different mcm

The only problem is this depends on the path from where the binary is being invoked, if its from the clustersim directory the above works but anywhere else it'll just be incorrect. This was the reason why I turned it into a flag (initially I was automatically computing the root path relative to the file)

Do you still think we should have the default?

aaronfern · 2026-09-30T08:19:49Z

+}
+
+func getClusterNames(cmd *cobra.Command) (clusters []string) {
+	clusters, _ = cmd.Flags().GetStringSlice("clusters")


Why ignore an error here?

aaronfern · 2026-09-30T08:20:08Z

+}
+
+func getClustersDir(cmd *cobra.Command) (dir string) {
+	dir, _ = cmd.Flags().GetString("dir")


Similar here too, why ignore an error?

aaronfern · 2026-09-30T08:27:38Z

+		// CopyShoot only works with one cluster at a time, to ensure user doesn't
+		// pass incorrect project when specifying multiple clusters. Multiple invocations
+		// are to be made in case data for multiple clusters needs to be fetched.
+		clusterNames := getClusterNames(cmd)


nit: why call this var clusterNames when you expect only one cluster name to be present?

Also, maybe this constraint should also be specified in README.md under the copyshoot section

aaronfern · 2026-09-30T09:34:16Z

+	Short:                 "Generates MachineClasses and MachineDeployments with the specified parameters.",
+	DisableFlagsInUseLine: true,
+	PreRunE: func(cmd *cobra.Command, _ []string) error {
+		if genClusterFlags.Zones == nil || genClusterFlags.Instances == nil {


I think you'll need a more sophisticated check here to ensure Zones or instances are not empty.
If I understood the docstring of MarkFlagRequired() correctly, it will just cause your command to report an error if invoked without the flag. So something like --zones "" would be legal.
This then passed the genClusterFlags.Zones == nil check since []Zones{""} will not be nil

Maybe something like:

for _, zone := range genClusterFlags.Zones { if strings.TrimSpace(zone) == "" { return fmt.Errorf("zone name cannot be empty") } }

aaronfern · 2026-09-30T09:38:49Z

+		&genClusterFlags.Zones, "zones", "z", nil,
+		"comma separated list of zones to use (required)",
+	)
+	_ = genClusterCmd.MarkFlagRequired("zones")


Returned error ignored again

aaronfern · 2026-09-30T09:39:01Z

+		&genClusterFlags.Instances, "instances", "i", nil,
+		"comma separated list of instances to use (required)",
+	)
+	_ = genClusterCmd.MarkFlagRequired("instances")


Returned error ignored

aaronfern · 2026-09-30T09:42:32Z

+			linkedConfigPath := filepath.Join(clusterPath, fileNameKubeconfig)
+			kubeConfig := clusterEnv.Cfg.KubeconfigFile()
+			// Clean up any existing, stale symlink
+			_ = os.Remove(linkedConfigPath)


Returned error ignored here

aaronfern · 2026-09-30T10:24:49Z

+				}
+			}
+
+			clusterEnv := cluster.New(name, dummyNamespace)


Why use a dummy namespace here?
You can fetch the proper namespace using getClusterNamespace(). If namespace does not matter then a comment explaining that would be needed.

elankath · 2026-10-06T07:26:12Z

 	@(cd test/integration/controller; ginkgo -v --show-node-events --poll-progress-after=300s --poll-progress-interval=60s)
+
+.PHONY: build-clustersim
+build-clustersim:


Can we please also duplicate this target (only this target in top level MCM Makefile)

elankath · 2026-10-06T07:29:22Z

+clustersim build --source-dir ../../../../ --components "mcm,mc"
+
+Flags:
+-c, --components strings   comma separated list of components to build (default [mcm,mc])


I suggest making a command build with different source directories passed --source-mcm and --source-ca. Since source directory can be different for each. Or ask user to specify env variables if this is too much work. I don't think building MC is necessary since it only builds the current simulated provider - which it should do by default for any build invocation using the --source-mcm directory and put built binaries into $source-mcm/bin.

elankath · 2026-10-06T07:29:57Z

+clustersim build {--source-dir ..} [--components mcm]...
+
+Examples:
+clustersim build --source-dir ../../../../ --components "mcm,mc"


It is not possible to build the ca tried

Error: passed invalid component "ca", supported ones are [mcm mc]

I had mentioned in the proposal that I didn't wish to add CA support for now. Wanted to keep the current task's scope restricted to just adding IT support and basic manual testing. This can be handled later unless you feel strongly that it should also be done now.

(I'd also mentioned skipping copyshoot and gendata for current implementation but since it was somewhat necessary for testing with gardener data, I added those subcommands)

elankath · 2026-10-06T07:37:29Z

+
+.PHONY: build-clustersim
+build-clustersim:
+	@mkdir -p bin


a bin directory for binaries should not go inside a pkg subdirectory in Go. You can use top level project bin directory for all binaries. There is no need to have gen/bin which is even more confusing as we now have 3 different build dirs. DOn't use cluster data directory to determine binary directory. Binaries are always at fixed location and constant.

elankath · 2026-10-06T07:45:32Z

+		return err
+	}
+
+	if finfo, err := os.Stat(binPath); err != nil && finfo.IsDir() {


error here: if err is not nil then finfo will be nil and you will get a panic.

elankath · 2026-10-06T07:47:58Z

+}
+
+func init() {
+	stopCmd.PersistentFlags().BoolVar(


Please move all flag setup code away from init. init runs at program startup BEFORE any flags are parsed. stopFlags.KillAll is always true here. Move to PersistentPreRun or alternative.

elankath · 2026-10-06T07:52:22Z

+					ctx, mcc.SecretRef.Name, mcc.SecretRef.Namespace, &secret,
+				)
+				if err != nil {
+					return false, err


This doesn't make sense. Shouldn't you do a IsNotFound check here ?

elankath · 2026-10-06T07:53:52Z

+	"embed"
+	"encoding/json"
+	"fmt"
+	"html/template"


Please use text/template. html/template is meant for HTML as it does HTML decode/encode stuff which can screw up yaml.

This was an LSP import, didn't pay attention. Thanks for the catch, will fix.

elankath · 2026-10-06T07:56:42Z

+		Use:                   "clustersim {--clusters name}... command [--dir /tmp]",
+		Short:                 "Manages and sets up virtual cluster running simulated Machine Controller Manager.",
+		Example:               `clustersim --clusters "test" setup`,
+		Args:                  cobra.ExactArgs(1),


Remove this. Has no relevance for sub-commands unless you have top-level argument for root command which you do not AFAIK.

elankath · 2026-10-06T08:02:17Z

@@ -0,0 +1,9249 @@
+[


This file is just too large at ~400KB. This is also not scalable when you need to simulate other provider instance types. We should not check it in. Just accept Accept capacity directly as flags. --cpu 2 --memory 8Gi --arch amd64 . Or some format. (check what other k8s projects do), instead of looking at it by instance name.

Yes, I was hesitant to add this, alternatively I thought I'll just populate the file with top 20 instances from each CSP but what you've proposed makes sense. I can introduce additional flags for the instance parameters.

elankath · 2026-10-06T08:13:28Z

+Generates MachineClasses and MachineDeployments with the specified parameters.
+
+Usage:
+clustersim gencluster {--clusters name} {--zones zone}... {--instances inst}...


I suggest supporting multiple --instance fields each with a spec like --instance m5.xlarge:amd64:2:0:8Gi . Order is name:arch:cpu:gpu:mem. Or more verbose like --instance name=m5.xlarge,cpu=2,mem=8Gi,arch=amd64, but I think this will give problem with cobra.

elankath · 2026-10-06T08:22:50Z

@@ -0,0 +1,130 @@
+## Clustersim
+


Please add pre-requisites for clustersim tool.

Error: kwok: failed to create cluster "aw1": exit status 1: exit status 1: {"time":"2026-10-06T13:51:50.793557+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":266},"msg":"Cluster is creating","cluster":"aw1"} {"time":"2026-10-06T13:51:51.18667+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/utils/file.getCacheOrDownload","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/utils/file/download.go","line":145},"msg":"Download","cluster":"aw1","uri":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver"} {"time":"2026-10-06T13:51:53.40609+05:30","level":"ERROR","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":269},"msg":"Failed to setup config","cluster":"aw1","err":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver: 404 Not Found"} {"time":"2026-10-06T13:51:53.415484+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE.func2","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":247},"msg":"Cluster is cleaned up","cluster":"aw1"} {"time":"2026-10-06T13:51:53.415536+05:30","level":"ERROR","source":{"function":"main.main","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/cmd/kwokctl/main.go","line":54},"msg":"Execute exit","err":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver: 404 Not Found"} Error: kwok: failed to create cluster "aw1": exit status 1: exit status 1: {"time":"2026-10-06T13:51:50.793557+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":266},"msg":"Cluster is creating","cluster":"aw1"} {"time":"2026-10-06T13:51:51.18667+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/utils/file.getCacheOrDownload","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/utils/file/download.go","line":145},"msg":"Download","cluster":"aw1","uri":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver"} {"time":"2026-10-06T13:51:53.40609+05:30","level":"ERROR","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":269},"msg":"Failed to setup config","cluster":"aw1","err":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver: 404 Not Found"} {"time":"2026-10-06T13:51:53.415484+05:30","level":"INFO","source":{"function":"sigs.k8s.io/kwok/pkg/kwokctl/cmd/create/cluster.runE.func2","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/pkg/kwokctl/cmd/create/cluster/cluster.go","line":247},"msg":"Cluster is cleaned up","cluster":"aw1"} {"time":"2026-10-06T13:51:53.415536+05:30","level":"ERROR","source":{"function":"main.main","file":"/Users/I034796/go/pkg/mod/sigs.k8s.io/kwok@v0.5.0/cmd/kwokctl/main.go","line":54},"msg":"Execute exit","err":"https://dl.k8s.io/release/v1.35.0/bin/darwin/arm64/kube-apiserver: 404 Not Found"}

This is because of old kwok version present on the system: 0.5.0

I'll add a note in the README to ensure that the user cleans it up and checks that the version is atleast 0.8.0.

Add clustersim command for manual mcm testing

33db0ca

Signed-off-by: Prashant Tak <prashant.tak@sap.com>

gardener-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 11, 2026

gardener-prow Bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 11, 2026

takoverflow added kind/feature and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Sep 11, 2026

takoverflow mentioned this pull request Sep 11, 2026

Introduce Simulation Provider for integration testing #1120

Open

1 of 4 tasks

takoverflow marked this pull request as ready for review September 15, 2026 19:54

takoverflow requested a review from a team as a code owner September 15, 2026 19:54

gardener-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 15, 2026

Add clustersim readme doc

ebfa590

Signed-off-by: Prashant Tak <prashant.tak@sap.com>

takoverflow force-pushed the feat/clustersim branch from 62d6692 to ebfa590 Compare September 15, 2026 20:01

aaronfern reviewed Sep 30, 2026

View reviewed changes

elankath reviewed Oct 6, 2026

View reviewed changes


		### `build`

		Builds the specified components, fallsback to building both `machine-controller-manager` and the `simulated-provider` if unspecifed. Requires a mandatory flag `source-dir` pointing to the `machine-controller-manager` source root directory.

Conversation

takoverflow commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gardener-prow Bot commented Sep 11, 2026

Uh oh!

aaronfern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elankath Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elankath Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elankath Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elankath Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elankath Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

takoverflow commented Sep 11, 2026 •

edited

Loading

elankath Oct 6, 2026 •

edited

Loading

elankath Oct 6, 2026 •

edited

Loading

elankath Oct 6, 2026 •

edited

Loading

elankath Oct 6, 2026 •

edited

Loading

elankath Oct 6, 2026 •

edited

Loading