[WIP] Initial PR for consuming gvproxy/macadam/cloud-init for crc-ng by praveenkumar · Pull Request #5025 · crc-org/crc

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.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 797da8c7-ca7b-49ff-b4f0-18d77540fed0

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

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

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

/retest

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

+	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:32:23Z

+			}
+			// 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:49Z

+		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

-			{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

+			{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:19:01Z

 	}
-	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 {


praveenkumar

Tried to update as per some suggestions and commented to some of the suggestions.

praveenkumar · 2026-01-30T11:15:59Z

+
+const userDataTemplate = `#cloud-config
+runcmd:
+  - systemctl enable --now kubelet


@cfergeau why? this is required because our bundle have kubelet service disabled by default.

praveenkumar · 2026-01-30T11:19:53Z

+  content: '%s'
+  owner: core
+  permissions: '0600'
+- path: /opt/crc/id_rsa.pub


no, /opt/crc/id_rsa.pub is the file which is required by mco ssh service https://github.com/crc-org/snc/blob/release-4.20/systemd/ocp-mco-sshkey.service here.

praveenkumar · 2026-01-30T11:23:32Z

+- path: /opt/crc/pass_developer
+  content: '%s'
+  permissions: '0644'
+- path: /opt/crc/ocp-custom-domain.service.done


Otherwise https://github.com/crc-org/snc/blob/release-4.20/systemd/ocp-custom-domain.service will fails and since it is Restart=on-failure so it is going to continue fail :( so better to have it as this magic file.

praveenkumar · 2026-01-30T16:14:41Z


+// ExecMainExitTimestamp returns the exit timestamp of a service's main process.
+// Returns empty string if the service has never run, otherwise returns the exit timestamp.
+func (c Commander) ExecMainExitTimestamp(name string) (string, error) {


It return value is something which can be parsed as time but our aim is if there is timestamp which means the service is ready run and exited

$ systemctl show --property "ExecMainExitTimestamp" --value systemd-binfmt.service Wed 2026-01-21 11:02:24 IST

openshift-ci · 2026-02-25T11:07:39Z

[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 ask for approval from praveenkumar. 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

Made-with: Cursor

Co-authored-by: Cursor <cursoragent@cursor.com> Made-with: Cursor

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

Fix a misleading error and make sure parent directory exist before generating new keypair otherwise error occur when try to write the keypair on a non exist path.

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 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. Co-authored-by: Cursor <cursoragent@cursor.com>

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. Co-authored-by: Cursor <cursoragent@cursor.com>

Made-with: Cursor

Introduce a new file containing the cursor commit rules for the project. This document outlines the guidelines for generating git commit messages, including the format, allowed commit types, and footer conventions. This addition aims to standardize commit practices across the codebase. Co-authored-by: Cursor <cursoragent@cursor.com>

Change the method of sourcing Go files in the Makefile to use the `find` command, excluding the vendor directories. Without this if we want to compile an uncommit source (mostly deleting a go file) the git ls-files still have that file reference and build fails very early. ``` $ git status On branch crc-ng Changes not staged for commit: (use "git add/rm <file>..." to update what will be committed) (use "git restore <file>..." to discard changes in working directory) modified: cmd/crc-embedder/cmd/embed.go deleted: pkg/crc/machine/libvirt/constants.go deleted: pkg/crc/machine/libvirt/driver_linux.go deleted: pkg/crc/machine/libvirt/templates_linux.go modified: pkg/crc/preflight/preflight_checks_linux.go modified: pkg/crc/preflight/preflight_linux.go modified: pkg/crc/preflight/preflight_linux_test.go no changes added to commit (use "git add" and/or "git commit -a") ✔ ~/work/github/crc [crc-ng L|● 3✚ 4⚑ 17] 12:31 $ make cross make: *** No rule to make target 'pkg/crc/machine/libvirt/constants.go', needed by 'out/macos-arm64/crc'. Stop. ```

Eliminate the libvirt package and its related files from the codebase, shifting the focus to qemu-kvm for virtualization checks as per macadam requirement. Update preflight checks to reflect this change, ensuring that the system verifies the presence of qemu-kvm instead of libvirt components. This refactor simplifies the code and aligns with the current macadam strategy. Co-authored-by: Cursor <cursoragent@cursor.com>

…d services Implement the WasSkippedDueToConditions method in the systemd Commander to check if service conditions are met. Update the WaitForServiceSuccessfullyFinished function to handle cases where services are skipped due to unmet conditions, indicating that their work was already completed in a previous run. This enhancement improves service management and logging clarity.

Since now macadam is used which by default using gvproxy so better to remove the virtual network implementation from the codebase.

Looks like our bundle now have service which automatic run growpart as part of cloud-init but then xfs_growfs is still required to grow xfs filesystem so we shouldn't return just after growpart and run xfs_growfs.

Update the removeCrcVM function to utilize macadam for VM status checks and removal. This change eliminates the previous libvirt dependency, streamlining the VM management process and improving error handling when removing the 'crc' VM.

As of now route-controller not able to expose the nodeport to host. We need to find a way for gvproxy side or route-conroller side to enable it.

It was initially added as part of ed9342 but since now we are moving to consume gvproxy and there is no virtual network running as part of crc daemon so this is no more required.

Since macadam right now not support resource resize as part of start => stop => resize resource => start so the test case is failing - crc-org/macadam#291

Since now openshift related changes shifted as part of the bundle which pass using cloud init file so changes to openshift side not going to work as part of stop => start. Test about developer user password change is moved to openshift feature where fresh crc instance is created.

Update EnsureVMPartitionSizeCorrect to use lsblk --bytes flag and parse sizes as byte values instead of relying on formatted strings with "G" suffix which fails because crc-ng create a disk for cloud init with size of 43K and parsing fails. This makes the parsing more reliable and consistent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

It was added before since there was no vsock support for 9p in windows, but since it is added so better to remove tcp fallback.

Generate and persist a hosts API token when the daemon starts and during preflight setup. Require the token as an `Authorization: Bearer` header on the `/hosts/add` and `/hosts/remove` admin-helper endpoints. Sync the same token into an OpenShift Secret so routes-controller can authenticate when updating the hosts file. Add unit tests for token creation and authorization checks.

openshift-ci · 2026-08-06T06:32:56Z

@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	`174cdf3`	link	false	`/test security`
ci/prow/e2e-microshift-crc	`174cdf3`	link	true	`/test e2e-microshift-crc`
ci/prow/e2e-crc	`174cdf3`	link	true	`/test e2e-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

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 force-pushed the crc-ng branch from 99bac6d to 4ad739c Compare December 17, 2025 10:12

praveenkumar force-pushed the crc-ng branch 5 times, most recently from 861432b to 7b070ed Compare January 8, 2026 10:35

praveenkumar force-pushed the crc-ng branch from 71d5530 to dbdcb99 Compare January 29, 2026 10:48

praveenkumar force-pushed the crc-ng branch from 7b070ed to ba5bda4 Compare January 30, 2026 07:08

praveenkumar commented Feb 2, 2026

View reviewed changes

praveenkumar force-pushed the crc-ng branch 2 times, most recently from 9c795f3 to e18270d Compare February 3, 2026 10:24

praveenkumar self-assigned this Feb 4, 2026

praveenkumar added this to Project planning: crc Feb 4, 2026

praveenkumar moved this to Work In Progress in Project planning: crc Feb 4, 2026

praveenkumar force-pushed the crc-ng branch from e18270d to 088561d Compare February 25, 2026 11:07

Cached gvproxy to consume with macadam

94b6da1

Made-with: Cursor

praveenkumar force-pushed the crc-ng branch from 87b3bfb to b092d00 Compare July 30, 2026 07:22

praveenkumar and others added 26 commits July 30, 2026 13:18

Cached macadam binary

85b6446

Co-authored-by: Cursor <cursoragent@cursor.com> Made-with: Cursor

Add Logic to have different filename for cache

5dcd27d

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

ssh: Ensure parent directory exist before keypair generation

45e161e

Fix a misleading error and make sure parent directory exist before generating new keypair otherwise error occur when try to write the keypair on a non exist path.

cluster: Don't apply pull secret patch

659f860

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.

systemd: Add result helper to get service exit result

978677e

systemd: Add helper to get service exit timestamp

56e69fd

vsock: Use gvclient to expose dns and ports

a3a0615

cluster: Add WaitForServiceSuccessfullyFinished helper

81a747e

This will help to check if the service if run and finished successfully or not before checking the details in the cluster. Co-authored-by: Cursor <cursoragent@cursor.com>

Fixup: fix unit test for macadam and preflight

0e959be

Made-with: Cursor

Daemon: Remove virtual network implementation

f31f855

Since now macadam is used which by default using gvproxy so better to remove the virtual network implementation from the codebase.

start: Don't return if growpart is run on existing grown disk

45ed555

Looks like our bundle now have service which automatic run growpart as part of cloud-init but then xfs_growfs is still required to grow xfs filesystem so we shouldn't return just after growpart and run xfs_growfs.

e2e: Remove libvirt and virsh specific tests

74d279b

e2e: Remove the nodeport test

63500c0

As of now route-controller not able to expose the nodeport to host. We need to find a way for gvproxy side or route-conroller side to enable it.

Preflight: remove check and fix for vsock

50e502e

It was initially added as part of ed9342 but since now we are moving to consume gvproxy and there is no virtual network running as part of crc daemon so this is no more required.

integration: Don't include resize test as part of integration suite

d4acfe6

Since macadam right now not support resource resize as part of start => stop => resize resource => start so the test case is failing - crc-org/macadam#291

daemon: Remove tcp fallback for 9p file sharing

58e1700

It was added before since there was no vsock support for 9p in windows, but since it is added so better to remove tcp fallback.

praveenkumar force-pushed the crc-ng branch from b092d00 to 174cdf3 Compare August 6, 2026 04:27

Uh oh!

Conversation

praveenkumar commented Nov 28, 2025

Uh oh!

coderabbitai Bot commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praveenkumar left a comment • 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!

Uh oh!

openshift-ci Bot commented Feb 25, 2026

Uh oh!

openshift-ci Bot commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Nov 28, 2025 •

edited

Loading

praveenkumar left a comment •

edited

Loading