[WIP] Initial PR for consuming gvproxy/macadam/cloud-init for crc-ng #5025

praveenkumar · 2025-11-28T10:37:16Z

No description provided.

coderabbitai · 2025-11-28T10:37:22Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2025-11-28T10:37:33Z

[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 evidolob 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

praveenkumar · 2025-12-16T15:04:34Z

/retest

cfergeau · 2025-12-16T16:29:27Z

pkg/crc/cache/cache.go

+	return newCache(constants.GvproxyPath(),
+		url,
+		version,
+		func(executable string) (string, error) {


NewAdminHelperCache uses exactly the same function, might be desirable to share that code.

Yes, we can share the code for getting the version info. May be a followup or new commit.

cfergeau · 2025-12-16T16:31:17Z

pkg/crc/preflight/preflight_checks_common.go

+		return errors.Wrap(err, "unexpected version of the gvproxy executable")
+	}
+	logging.Debug("gvproxy executable already cached")
+	// SUID is only required on Linux (checkSuid is a no-op on Windows/macOS)


Are you sure it’s not also needed on macOS? Could be better to split this check in 2, with the SUID check being linux-specific. We would get 2 separate skip- options, which would allow to skip one without skipping the other (eg skip the suid check, but keep the gvproxy download).

This is removed in later commits

we only needed the capability instead of SUID

cfergeau · 2025-12-16T16:32:23Z

pkg/crc/cache/cache.go

+			}
+			// macadam version output format: "macadam version v0.2.0"
+			split := strings.Split(out, " ")
+			return strings.TrimSpace(split[len(split)-1]), nil


Same comment as for NewAdminHelperCache and NewGvproxyCache

cfergeau · 2025-12-16T16:33:00Z

pkg/crc/constants/constants.go

 	CrcLandingPageURL         = "https://console.redhat.com/openshift/create/local" // #nosec G101
 	DefaultAdminHelperURLBase = "https://github.com/crc-org/admin-helper/releases/download/v%s/%s"
 	DefaultGvproxyURLBase     = "https://github.com/containers/gvisor-tap-vsock/releases/download/%s/%s"
+	DefaultMacadamURLBase     = "https://github.com/praveenkumar/macadam/releases/download/%s/%s"


This can be changed now that there is a 0.3.0 release.

cfergeau · 2025-12-16T16:33:49Z

pkg/crc/preflight/preflight_checks_common.go

+		return errors.New("macadam executable is not cached")
+	}
+	if err := macadam.CheckVersion(); err != nil {
+		return errors.Wrap(err, "unexpected version of the macadam executable")


Let’s not add new uses of errors.Wrap in new code.
https://github.com/pkg/errors

This repository was archived by the owner on Dec 1, 2021. It is now read-only.

cfergeau · 2025-12-16T17:09:13Z

pkg/crc/machine/vsock.go

-			{Name: "api-int", IP: "192.168.127.2"},
-			{Name: "crc", IP: "192.168.126.11"},
+		Records: []types.Record{
+			{Name: "host", IP: net.ParseIP("192.168.127.254"), Regexp: nil},


Is it required to set Regexp: nil?

cfergeau · 2025-12-16T17:09:31Z

pkg/crc/machine/vsock.go

+			{Name: "api-int", IP: net.ParseIP("192.168.127.2"), Regexp: nil},
+			{Name: "crc", IP: net.ParseIP("192.168.126.11"), Regexp: nil},
 		},
+		DefaultIP: net.ParseIP("192.168.127.2"),


DefaultIP was not there before.

cfergeau · 2025-12-16T17:12:11Z

pkg/crc/machine/vsock.go

-	}
-	for i := range missingPorts {
-		port := &missingPorts[i]
-		if err := daemonClient.NetworkClient.Expose(port); err != nil {


Shouldn’t this logic be kept?

cfergeau · 2025-12-16T17:13:05Z

pkg/crc/machine/vsock.go

 	return false
 }

-func unexposePorts() error {


It’s used in Stop and Delete

since now it is manage by gvproxy process which get killed and release ports when crc stop/delete happen.

cfergeau · 2025-12-16T17:19:01Z

pkg/crc/machine/start.go

 	}
-	if err := cluster.EnsureGeneratedClientCAPresentInTheCluster(ctx, ocConfig, sshRunner, selfSignedCACert, adminClientCA); err != nil {
-		return errors.Wrap(err, "Failed to update user CA to cluster")
+	if err := sshRunner.CopyFileFromVM("/opt/kubeconfig", kubeconfigFilePath, 0644); err != nil {


This will help to use gvproxy as filename instead gvproxy-linux-amd64

Since with self sufficient bundle and cloud-init metadata pull secret should be applied automatic so just verify it instead apply. If it is not there that means something wrong with pull secret service in the bundle.

This helper can use to check if net cap is part of a binary or not for linux and implement no-ops for win and darwin

This will help to bind ports which are <1024 in Linux.

This commit have following - macadam helper to provide value based on macadam command run - cloud init helper to create user metadata - remove of libmachine api - updating vsock to work with gvproxy socket - Some todo which need to be improved

This will help to check if the service if run and finished successfully or not before checking the details in the cluster.

With self sufficient bundle all the ca changes happen as part of service run in the VM so once that service finish successfully it is easy to just copy that kubeconfig file from VM to host.

openshift-ci · 2025-12-17T11:08:03Z

@praveenkumar: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/security	`4ad739c`	link	false	`/test security`
ci/prow/e2e-microshift-crc	`4ad739c`	link	true	`/test e2e-microshift-crc`
ci/prow/e2e-crc	`4ad739c`	link	true	`/test e2e-crc`
ci/prow/integration-crc	`4ad739c`	link	true	`/test integration-crc`

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci bot added the do-not-merge/work-in-progress label Nov 28, 2025

openshift-ci bot requested review from albfan and evidolob November 28, 2025 10:37

anjannath mentioned this pull request Dec 8, 2025

[wip] disable post-start functions that are done in snc #4560

Closed

8 tasks

praveenkumar force-pushed the crc-ng branch 2 times, most recently from 49de216 to 2cf49bb Compare December 16, 2025 10:22

openshift-merge-robot added the needs-rebase label Dec 16, 2025

Cached gvproxy to consume with macadam

5536757

praveenkumar force-pushed the crc-ng branch from 2cf49bb to a7b9a83 Compare December 16, 2025 10:28

openshift-merge-robot removed the needs-rebase label Dec 16, 2025

praveenkumar force-pushed the crc-ng branch from a7b9a83 to bcd6e28 Compare December 16, 2025 11:09

praveenkumar force-pushed the crc-ng branch from bcd6e28 to 99bac6d Compare December 16, 2025 15:09

cfergeau reviewed Dec 16, 2025

View reviewed changes

praveenkumar added 14 commits December 17, 2025 13:18

Cached macadam binary

f83b7e3

Add Logic to have different filename for cache

6655bd8

This will help to use gvproxy as filename instead gvproxy-linux-amd64

ssh: Ensure parent directory exist before keypair generation

cae9d91

cluster: Don't apply pull secret patch

248f153

Since with self sufficient bundle and cloud-init metadata pull secret should be applied automatic so just verify it instead apply. If it is not there that means something wrong with pull secret service in the bundle.

Preflight: Create helper for set and get the capabilities

ba106c2

This helper can use to check if net cap is part of a binary or not for linux and implement no-ops for win and darwin

Preflight: add net capability instead suid for gvproxy

7289574

This will help to bind ports which are <1024 in Linux.

Update OCP bundle to 4.20.5 (stable channel)

a4edee5

systemd: Add result helper to get service exit result

d42eded

systemd: Add helper to get service exit timestamp

89e8e39

vsock: Use gvclient to expose dns and ports

1b3ef72

cluster: Add WaitForServiceSuccessfullyFinished helper

21d10dc

This will help to check if the service if run and finished successfully or not before checking the details in the cluster.

start: Copy the kubeconfig file from VM to host instead update

8167f17

With self sufficient bundle all the ca changes happen as part of service run in the VM so once that service finish successfully it is easy to just copy that kubeconfig file from VM to host.

Fixup: fix unit test for macadam and preflight

19bba9d

Fixup: make golint happy

4ad739c

praveenkumar force-pushed the crc-ng branch from 99bac6d to 4ad739c Compare December 17, 2025 10:12

[WIP] Initial PR for consuming gvproxy/macadam/cloud-init for crc-ng #5025

Are you sure you want to change the base?

[WIP] Initial PR for consuming gvproxy/macadam/cloud-init for crc-ng #5025

Uh oh!

Conversation

praveenkumar commented Nov 28, 2025

Uh oh!

coderabbitai bot commented Nov 28, 2025

Review skipped

Uh oh!

openshift-ci bot commented Nov 28, 2025

Uh oh!

praveenkumar commented Dec 16, 2025

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!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants