DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #359

vishesh92 · 2024-05-24T11:33:57Z

Recreated PR #328 by @hrak with minor fixups

Issue #, if available:

Description of changes:

This is a breaking change in the (unreleased) v1beta3 API which changes the DiskOffering property of CloudStackMachine to be a pointer to CloudStackResourceDiskOffering

Reasoning is described in the above linked issue.

Testing performed:

make test

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

netlify · 2024-05-24T11:34:13Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-cloudstack ready!

Name	Link
🔨 Latest commit	`932469e`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-cloudstack/deploys/665a2d7aff90370008f73c87
😎 Deploy Preview	https://deploy-preview-359--kubernetes-sigs-cluster-api-cloudstack.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

vishesh92 · 2024-05-24T11:49:13Z

/ok-to-test

blueorangutan · 2024-05-24T14:20:59Z

Test Results : (tid-434)
Environment: kvm Rocky8(x3), Advanced Networking with Management Server Rocky8
Kubernetes Version: v1.27.2
Kubernetes Version upgrade from: v1.26.5
Kubernetes Version upgrade to: v1.27.2
CloudStack Version: 4.19
Template: ubuntu-2004-kube
E2E Test Run Logs: https://github.com/blueorangutan/capc-prs/releases/download/capc-pr-ci-cd/capc-e2e-artifacts-pr359-sl-434.zip



Summarizing 3 Failures:
 [FAIL] When testing affinity group [It] Should have host affinity group when affinity is pro
 /jenkins/workspace/capc-e2e-new/test/e2e/common.go:332
 [FAIL] When testing resource cleanup [BeforeEach] Should create a new network when the specified network does not exist
 /jenkins/workspace/capc-e2e-new/test/e2e/resource_cleanup.go:62
 [FAIL] When testing app deployment to the workload cluster with slow network [ToxiProxy] [It] Should be able to download an HTML from the app deployed to the workload cluster
 /jenkins/workspace/capc-e2e-new/test/e2e/deploy_app_toxi.go:135

Ran 28 of 29 Specs in 9415.561 seconds
FAIL! -- 25 Passed | 3 Failed | 0 Pending | 1 Skipped
--- FAIL: TestE2E (9415.56s)
FAIL

rohityadavcloud · 2024-05-24T16:07:19Z

/approve
/retest

weizhouapache · 2024-05-28T12:45:56Z

/approve
/run-e2e -c 4.19

k8s-ci-robot · 2024-05-28T12:46:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rohityadavcloud, vishesh92, weizhouapache

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rohityadavcloud,vishesh92,weizhouapache]

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

blueorangutan · 2024-05-28T12:46:04Z

@weizhouapache a jenkins job has been kicked to run test with following paramaters:

kubernetes version: 1.27.2
CloudStack version: 4.19
hypervisor: kvm
template: ubuntu-2004-kube
Kubernetes upgrade from: 1.26.5 to 1.27.2

blueorangutan · 2024-05-28T17:53:25Z

Setting up environment failed

blueorangutan · 2024-05-29T08:30:52Z

Test Results : (tid-445)
Environment: kvm Rocky8(x3), Advanced Networking with Management Server Rocky8
Kubernetes Version: v1.27.2
Kubernetes Version upgrade from: v1.26.5
Kubernetes Version upgrade to: v1.27.2
CloudStack Version: 4.19
Template: ubuntu-2004-kube
E2E Test Run Logs: https://github.com/blueorangutan/capc-prs/releases/download/capc-pr-ci-cd/capc-e2e-artifacts-pr359-sl-445.zip



Summarizing 2 Failures:
 [FAIL] When testing project [AfterEach] Should create a cluster in a project
 /jenkins/workspace/capc-e2e-new/test/e2e/project.go:103
 [FAIL] When testing affinity group [It] Should have host affinity group when affinity is pro
 /jenkins/workspace/capc-e2e-new/test/e2e/common.go:348

Ran 29 of 30 Specs in 9232.045 seconds
FAIL! -- 27 Passed | 2 Failed | 0 Pending | 1 Skipped
--- FAIL: TestE2E (9232.05s)
FAIL

g-gaston

It's possible that I missed something, but isn't v1beta3 already released? If this is a breaking change, don't we need v1beta4?

vishesh92 · 2024-05-31T06:23:04Z

It's possible that I missed something, but isn't v1beta3 already released? If this is a breaking change, don't we need v1beta4?

As per my understanding, this will allow users migrate to v1beta3 without any loss in metadata when they are upgrading CAPC.

Signed-off-by: Hans Rakers <[email protected]>

In accordance with https://release-0-4.cluster-api.sigs.k8s.io/developer/providers/v1alpha2-to-v1alpha3#add-matchpolicyequivalent-kubebuilder-marker-in-webhooks Signed-off-by: Hans Rakers <[email protected]>

blueorangutan · 2024-06-01T03:21:35Z

Test Results : (tid-466)
Environment: kvm Rocky8(x3), Advanced Networking with Management Server Rocky8
Kubernetes Version: v1.27.2
Kubernetes Version upgrade from: v1.26.5
Kubernetes Version upgrade to: v1.27.2
CloudStack Version: 4.19
Template: ubuntu-2004-kube
E2E Test Run Logs: https://github.com/blueorangutan/capc-prs/releases/download/capc-pr-ci-cd/capc-e2e-artifacts-pr359-sl-466.zip



Summarizing 3 Failures:
 [FAIL] When testing app deployment to the workload cluster with slow network [ToxiProxy] [It] Should be able to download an HTML from the app deployed to the workload cluster
 /jenkins/workspace/capc-e2e-new/test/e2e/deploy_app_toxi.go:135
 [FAIL] When testing affinity group [It] Should have host affinity group when affinity is pro
 /jenkins/workspace/capc-e2e-new/test/e2e/common.go:348
 [FAIL] When testing project [AfterEach] Should create a cluster in a project
 /jenkins/workspace/capc-e2e-new/test/e2e/project.go:103

Ran 29 of 30 Specs in 10007.088 seconds
FAIL! -- 26 Passed | 3 Failed | 0 Pending | 1 Skipped
--- FAIL: TestE2E (10007.09s)
FAIL

g-gaston

lgtm
@vignesh-goutham for final approval since he had more context on v1beta3

vignesh-goutham · 2024-06-13T15:49:51Z

It's possible that I missed something, but isn't v1beta3 already released? If this is a breaking change, don't we need v1beta4?

As per my understanding, this will allow users migrate to v1beta3 without any loss in metadata when they are upgrading CAPC.

Could you help me understand how this will work? Looking at the code, there are conversions to and from v1beta3 to other versions. An existing CloudStackMachine on v1beta3 cant mutate to use a pointer to diskOffering. Its recommended to do only additive changes to released APIs, this seems like it might not be additive in nature, unless I'm missing something.

vishesh92 · 2024-06-18T07:45:04Z

It's possible that I missed something, but isn't v1beta3 already released? If this is a breaking change, don't we need v1beta4?

As per my understanding, this will allow users migrate to v1beta3 without any loss in metadata when they are upgrading CAPC.

Could you help me understand how this will work? Looking at the code, there are conversions to and from v1beta3 to other versions. An existing CloudStackMachine on v1beta3 cant mutate to use a pointer to diskOffering. Its recommended to do only additive changes to released APIs, this seems like it might not be additive in nature, unless I'm missing something.

@hrak was the original author of the PR. So, he would have more and better context about this PR. I recreated the PR to fix some minor fixes and conflicts.

I just went through the PR again to understand the changes. This PR is to fix #326 and PR #332 fixed the metadata across different versions.

k8s-triage-robot · 2024-10-02T12:21:26Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

vishesh92 · 2024-10-25T05:23:15Z

@vignesh-goutham let me know if we can merge this?

k8s-triage-robot · 2024-11-24T06:17:09Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-ci-robot requested review from davidjumani and weizhouapache May 24, 2024 11:34

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 24, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 24, 2024

vishesh92 changed the title ~~Bug/disk offering ptr~~ DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering May 24, 2024

vishesh92 mentioned this pull request May 24, 2024

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #328

Closed

vishesh92 linked an issue May 24, 2024 that may be closed by this pull request

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #326

Closed

vishesh92 requested review from rohityadavcloud, g-gaston and hrak and removed request for davidjumani May 24, 2024 11:48

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 24, 2024

vishesh92 force-pushed the bug/disk_offering_ptr branch from 01b9efd to fad32bc Compare May 29, 2024 05:50

g-gaston reviewed May 30, 2024

View reviewed changes

rohityadavcloud added this to the v0.5.0 milestone May 31, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2024

hrak added 3 commits June 1, 2024 01:34

DiskOffering is optional and thus should be a ptr

9a0132c

Signed-off-by: Hans Rakers <[email protected]>

Fix conversion-gen

3b3fec0

Signed-off-by: Hans Rakers <[email protected]>

Set matchPolicy: Equivalent and update webhook names

8dc59b2

In accordance with https://release-0-4.cluster-api.sigs.k8s.io/developer/providers/v1alpha2-to-v1alpha3#add-matchpolicyequivalent-kubebuilder-marker-in-webhooks Signed-off-by: Hans Rakers <[email protected]>

Fixup

932469e

vishesh92 force-pushed the bug/disk_offering_ptr branch from fad32bc to 932469e Compare May 31, 2024 20:05

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2024

g-gaston reviewed Jun 4, 2024

View reviewed changes

vishesh92 modified the milestones: v0.5.0, v0.6 Jul 4, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 2, 2024

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #359

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #359

vishesh92 commented May 24, 2024 •

edited

Loading

netlify bot commented May 24, 2024 •

edited

Loading

vishesh92 commented May 24, 2024

blueorangutan commented May 24, 2024

rohityadavcloud commented May 24, 2024

weizhouapache commented May 28, 2024

k8s-ci-robot commented May 28, 2024

blueorangutan commented May 28, 2024

blueorangutan commented May 28, 2024

blueorangutan commented May 29, 2024

g-gaston left a comment

vishesh92 commented May 31, 2024

blueorangutan commented Jun 1, 2024

g-gaston left a comment

vignesh-goutham commented Jun 13, 2024

vishesh92 commented Jun 18, 2024

k8s-triage-robot commented Oct 2, 2024

vishesh92 commented Oct 25, 2024

k8s-triage-robot commented Nov 24, 2024

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #359

Are you sure you want to change the base?

DiskOffering in CloudStackMachine CRD should be a pointer to CloudStackResourceDiskOffering #359

Conversation

vishesh92 commented May 24, 2024 • edited Loading

netlify bot commented May 24, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-cluster-api-cloudstack ready!

vishesh92 commented May 24, 2024

blueorangutan commented May 24, 2024

rohityadavcloud commented May 24, 2024

weizhouapache commented May 28, 2024

k8s-ci-robot commented May 28, 2024

blueorangutan commented May 28, 2024

blueorangutan commented May 28, 2024

blueorangutan commented May 29, 2024

g-gaston left a comment

Choose a reason for hiding this comment

vishesh92 commented May 31, 2024

blueorangutan commented Jun 1, 2024

g-gaston left a comment

Choose a reason for hiding this comment

vignesh-goutham commented Jun 13, 2024

vishesh92 commented Jun 18, 2024

k8s-triage-robot commented Oct 2, 2024

vishesh92 commented Oct 25, 2024

k8s-triage-robot commented Nov 24, 2024

vishesh92 commented May 24, 2024 •

edited

Loading

netlify bot commented May 24, 2024 •

edited

Loading