search

kubernetes-sigs / cluster-api-provider-cloudstack

A Kubernetes Cluster API Provider implementation for Apache CloudStack.
https://cluster-api-cloudstack.sigs.k8s.io/
Apache License 2.0
37 stars 35 forks source link

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

Open vishesh92 opened 6 months ago

vishesh92 commented 6 months ago

Recreated PR https://github.com/kubernetes-sigs/cluster-api-provider-cloudstack/pull/328 by @hrak with minor fixups

Issue #, if available:

326

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[bot] commented 6 months ago

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

Name Link
Latest commit 932469e2f0fa128491de57014c3dd74e4d253c9c
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...

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 commented 6 months ago

/ok-to-test

blueorangutan commented 6 months ago

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 commented 6 months ago

/approve /retest

weizhouapache commented 6 months ago

/approve /run-e2e -c 4.19

k8s-ci-robot commented 6 months ago

[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](https://github.com/kubernetes-sigs/cluster-api-provider-cloudstack/blob/main/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 commented 6 months ago

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

blueorangutan commented 6 months ago

Setting up environment failed

blueorangutan commented 6 months ago

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
vishesh92 commented 6 months ago

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.

blueorangutan commented 6 months ago

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
vignesh-goutham commented 5 months ago

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 commented 5 months ago

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 https://github.com/kubernetes-sigs/cluster-api-provider-cloudstack/pull/332 fixed the metadata across different versions.

k8s-triage-robot commented 1 month ago

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

This bot triages PRs according to the following rules:

You can:

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

/lifecycle stale

vishesh92 commented 1 month ago

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

k8s-triage-robot commented 4 days ago

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

This bot triages PRs according to the following rules:

You can:

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

/lifecycle rotten