Skip to content

Add support for linked clones #478

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

Merged
merged 4 commits into from
May 9, 2025
Merged

Conversation

LukasK32
Copy link
Contributor

Issue #, if available:
#174

Description of changes:

  • Fix ProxmoxMachineSpec validation rules
    • Previous rule required full to be always set to true and did not validate for storage being set
  • Move validation rules from ProxmoxMachine to VirtualMachineCloneSpec
    • Moving validation rules to VirtualMachineCloneSpec also adds them to other resources where it is embedded that users interact with directly
  • Remove default value from VirtualMachineCloneSpec.Format
    • Having format default to raw prevents making linked clones on storages that support it

Testing performed:

  • make test
  • Manually deploying multiple clusters with and without linked clones enabled

Copy link

Tests

Please note that running unit and e2e tests requires manual approval from a team member.

e2e tests

We use labels to control which e2e tests contexts are run:

Label Behaviour
none Run Generic tests only
e2e/none skip all e2e tests (documentation etc) - overrides all e2e/* labels Do not run any e2e tests
e2e/flatcar run Flatcar e2e tests Add Flatcar tests

ℹ️ Ask a team member to add the requested labels if you don't have enough permissions.

Copy link
Member

@mcbenjemaa mcbenjemaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your contribution.

Have you tested the functionality of the provider while setting linked clones?


Expect(k8sClient.Create(context.Background(), dm)).Should(MatchError(ContainSubstring("Must set full=true when specifying storage")))
})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case for a linked clone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added

@LukasK32
Copy link
Contributor Author

Have you tested the functionality of the provider while setting linked clones?

Yes, tested both using linked clones as well as full clones

@mcbenjemaa
Copy link
Member

Please Rebase and we are good to go.

LukasK32 added 4 commits May 6, 2025 14:06
Previous rule required 'full' to be always set to 'true' and did not validate for 'storage' being set.

Signed-off-by: Lukas Kirylak <[email protected]>
Moving validation rules to VirtualMachineCloneSpec also adds them to other resources where it is embedded that users interact with directly.

Signed-off-by: Lukas Kirylak <[email protected]>
Having 'format' default to 'raw' prevents making linked clones on storages that support it.

Signed-off-by: Lukas Kirylak <[email protected]>
@LukasK32
Copy link
Contributor Author

LukasK32 commented May 6, 2025

Rebased 👍

@mcbenjemaa mcbenjemaa enabled auto-merge (squash) May 6, 2025 14:38
Copy link

sonarqubecloud bot commented May 9, 2025

@mcbenjemaa mcbenjemaa merged commit 679ebc1 into ionos-cloud:main May 9, 2025
10 checks passed
@mcbenjemaa mcbenjemaa mentioned this pull request May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants