-
Notifications
You must be signed in to change notification settings - Fork 257
[WIP] Initial PR for consuming gvproxy/macadam/cloud-init for crc-ng #5025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: crc-ng
Are you sure you want to change the base?
Changes from all commits
b2e2c14
8a8f59e
90611e0
47f291a
90bee86
afaf6f5
93b1008
6125fb7
a7ef31f
4efe7a7
35ec486
5c38796
a441f16
84e24da
1e46a51
f8a9059
4135f8d
7308267
e18270d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # Cursor Commit Rules for crc-org/crc | ||
|
|
||
| When generating git commit messages: | ||
| - Follow the Conventional Commits specification format: | ||
| <type>[optional scope]: <subject> | ||
| [blank line] | ||
| [optional body explaining what changed and why] | ||
| [optional footer with references, breaking changes] | ||
| - Use **imperative** and **present tense** in the subject line. | ||
| - Limit the **subject line to ≈50–72 characters**; wrap body at ≈72. | ||
| - Use meaningful scopes when it clarifies affected modules (e.g., `pkg`, `cmd`, `docs`, `test`, `tools`). | ||
| - Prefer concise yet descriptive wording of **what changed** and **why**, not just **how**. | ||
| - When applicable, reference related issues or pull request numbers in footer. | ||
|
|
||
| # Allowed Commit Types | ||
| feat: add user-visible functionality or significant enhancements | ||
| fix: bug fix | ||
| docs: documentation only (e.g., README, docs/) | ||
| style: formatting, whitespace, linting (no code logic change) | ||
| refactor: code change that neither fixes a bug nor adds a feature | ||
| perf: performance improvement | ||
| test: adding or fixing tests | ||
| ci: continuous integration/config change (e.g., GitHub Actions) | ||
| build: build system or external dependency changes | ||
| chore: routine tasks without code change (scripts, config) | ||
|
|
||
| # Type Rules | ||
| feat: | ||
| - New CLI subcommands or flags | ||
| - New OpenShift, MicroShift, or podman integration options | ||
|
|
||
| fix: | ||
| - Correct unintended behaviors in cluster start/stop logic | ||
| - Resolve crashes, error cases, CLI UX bugs | ||
|
|
||
| docs: | ||
| - Add or update CRC documentation pages | ||
| - Fix README typos or incorrect instructions | ||
|
|
||
| style: | ||
| - Adjust formatting (go fmt), remove unused imports | ||
| - Linters fixes; no functional impact | ||
|
|
||
| refactor: | ||
| - Extract utils, reorganize package structure | ||
| - Rename internal functions for clarity | ||
|
|
||
| perf: | ||
| - Optimize CPU/memory usage in cluster operations | ||
|
|
||
| test: | ||
| - Add or improve unit/integration tests | ||
| - Update CI test matrix | ||
|
|
||
| ci: | ||
| - CI workflow fixes or adjustments (e.g., GitHub workflows) | ||
|
|
||
| build: | ||
| - Update Go module versions | ||
| - Changes to build scripts (Makefile, tooling) | ||
|
|
||
| chore: | ||
| - Non-code housekeeping (dependency cleanup, templating) | ||
|
|
||
| # Footer Conventions | ||
| - Close issues with “Closes #<number>.” | ||
| - Denote breaking changes via `BREAKING CHANGE:` in body/footer. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ type Cache struct { | |
| version string | ||
| ignoreNameMismatch bool | ||
| getVersion func(string) (string, error) | ||
| targetName string // Optional: if set, rename the executable to this name | ||
| } | ||
|
|
||
| type VersionMismatchError struct { | ||
|
|
@@ -84,6 +85,44 @@ func NewAdminHelperCache() *Cache { | |
| ) | ||
| } | ||
|
|
||
| func NewGvproxyCache() *Cache { | ||
| url := constants.GetGvproxyURL() | ||
| version := version.GetGvproxyVersion() | ||
| cache := newCache(constants.GvproxyPath(), | ||
| url, | ||
| version, | ||
| func(executable string) (string, error) { | ||
| out, _, err := crcos.RunWithDefaultLocale(executable, "--version") | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| // gvproxy --version output format: "gvproxy version v0.8.7" | ||
| split := strings.Split(out, " ") | ||
| return strings.TrimSpace(split[len(split)-1]), nil | ||
| }, | ||
| ) | ||
| cache.targetName = constants.GetGvproxyExecutableName() | ||
| return cache | ||
| } | ||
|
|
||
| func NewMacadamCache() *Cache { | ||
| url := constants.GetMacadamURL() | ||
| version := version.GetMacadamVersion() | ||
| return newCache(constants.MacadamPath(), | ||
| url, | ||
| version, | ||
| func(executable string) (string, error) { | ||
| out, _, err := crcos.RunWithDefaultLocale(executable, "--version") | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| // macadam version output format: "macadam version v0.2.0" | ||
| split := strings.Split(out, " ") | ||
| return strings.TrimSpace(split[len(split)-1]), nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as for |
||
| }, | ||
| ) | ||
| } | ||
|
|
||
| func (c *Cache) IsCached() bool { | ||
| if _, err := os.Stat(c.GetExecutablePath()); os.IsNotExist(err) { | ||
| return false | ||
|
|
@@ -136,7 +175,12 @@ func (c *Cache) cacheExecutable() error { | |
|
|
||
| // Copy the requested asset into its final destination | ||
| for _, extractedFilePath := range extractedFiles { | ||
| finalExecutablePath := filepath.Join(constants.CrcBinDir, c.GetExecutableName()) | ||
| // Use targetName if set, otherwise use the original executable name | ||
| finalName := c.GetExecutableName() | ||
| if c.targetName != "" { | ||
| finalName = c.targetName | ||
| } | ||
| finalExecutablePath := filepath.Join(constants.CrcBinDir, finalName) | ||
| // If the file exists then remove it (ignore error) first before copy because with `0500` permission | ||
| // it is not possible to overwrite the file. | ||
| os.Remove(finalExecutablePath) | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| package cloudinit | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
|
|
||
| "github.com/crc-org/crc/v2/pkg/crc/constants" | ||
| ) | ||
|
|
||
| // UserDataOptions contains all the options needed to generate cloud-init user-data | ||
| type UserDataOptions struct { | ||
| PublicKey string | ||
| PullSecret string | ||
| KubeAdminPassword string | ||
| DeveloperPassword string | ||
| } | ||
|
|
||
| const userDataTemplate = `#cloud-config | ||
| runcmd: | ||
| - systemctl enable --now kubelet | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And ideally this would also not be needed…
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cfergeau why? this is required because our bundle have kubelet service disabled by default.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same answer as #5025 (comment) |
||
| write_files: | ||
| - path: /home/core/.ssh/authorized_keys | ||
| content: '%s' | ||
| owner: core | ||
| permissions: '0600' | ||
| - path: /opt/crc/id_rsa.pub | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this go in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a document for the cloud-init file we expect, together with an explanation about what uses each cloud-init "parameter"? |
||
| content: '%s' | ||
| owner: root:root | ||
| permissions: '0644' | ||
| - path: /etc/sysconfig/crc-env | ||
| content: | | ||
| CRC_SELF_SUFFICIENT=1 | ||
| CRC_NETWORK_MODE_USER=1 | ||
| owner: root:root | ||
| permissions: '0644' | ||
| - path: /opt/crc/pull-secret | ||
| content: | | ||
| %s | ||
| permissions: '0644' | ||
| - path: /opt/crc/pass_kubeadmin | ||
| content: '%s' | ||
| permissions: '0644' | ||
| - path: /opt/crc/pass_developer | ||
| content: '%s' | ||
| permissions: '0644' | ||
| - path: /opt/crc/ocp-custom-domain.service.done | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally this would not be needed… It’s when we want a custom domain that something would appear in the cloud-init file. But for the nominal case, we should not have to add magic files.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise https://github.com/crc-org/snc/blob/release-4.20/systemd/ocp-custom-domain.service will fails and since it is
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point is that this would be a nice improvement to make to the self sufficient bundle… The less magic is required in the cloud-init file, the better |
||
| permissions: '0644' | ||
| ` | ||
|
|
||
| // compactJSON compacts a JSON string by removing whitespace and newlines | ||
| func compactJSON(jsonStr string) (string, error) { | ||
| var buf bytes.Buffer | ||
| if err := json.Compact(&buf, []byte(jsonStr)); err != nil { | ||
| return "", fmt.Errorf("failed to compact JSON: %w", err) | ||
| } | ||
| return buf.String(), nil | ||
| } | ||
|
|
||
| // GenerateUserData generates a cloud-init user-data file and returns the path | ||
| func GenerateUserData(machineName string, opts UserDataOptions) (string, error) { | ||
| // Create the machine directory if it doesn't exist | ||
| machineDir := filepath.Dir(getUserDataPath(machineName)) | ||
| if err := os.MkdirAll(machineDir, 0o750); err != nil { | ||
| return "", fmt.Errorf("failed to create machine directory: %w", err) | ||
| } | ||
|
|
||
| // Compact the pull secret JSON | ||
| compactPullSecret, err := compactJSON(opts.PullSecret) | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to compact pull secret: %w", err) | ||
| } | ||
|
|
||
| // Generate the cloud-init user-data content | ||
| userData := fmt.Sprintf(userDataTemplate, | ||
| opts.PublicKey, // /home/core/.ssh/authorized_keys | ||
| opts.PublicKey, // /opt/crc/id_rsa.pub | ||
| compactPullSecret, // /opt/crc/pull-secret (compacted) | ||
| opts.KubeAdminPassword, // /opt/crc/pass_kubeadmin | ||
| opts.DeveloperPassword, // /opt/crc/pass_developer | ||
| ) | ||
|
|
||
| userDataPath := getUserDataPath(machineName) | ||
| // Write the user-data file | ||
| if err := os.WriteFile(userDataPath, []byte(userData), 0o600); err != nil { | ||
| return "", fmt.Errorf("failed to write user-data file: %w", err) | ||
| } | ||
|
|
||
| return userDataPath, nil | ||
| } | ||
|
|
||
| // RemoveUserData removes the cloud-init user-data file for a machine | ||
| func RemoveUserData(machineName string) error { | ||
| userDataPath := filepath.Join(constants.MachineInstanceDir, machineName, "user-data") | ||
| if err := os.Remove(userDataPath); err != nil && !os.IsNotExist(err) { | ||
| return fmt.Errorf("failed to remove user-data file: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // getUserDataPath returns the path to the user-data file for a machine | ||
| func getUserDataPath(machineName string) string { | ||
| return filepath.Join(constants.MachineInstanceDir, machineName, "user-data") | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NewAdminHelperCacheuses exactly the same function, might be desirable to share that code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can share the code for getting the version info. May be a followup or new commit.