Skip to content

Conversation

@elmiko
Copy link
Contributor

@elmiko elmiko commented Jan 21, 2022

This change updates the alibaba provider spec usage related to the
vswitch, security groups, and resource group. The API for the provider
spec is changing to use a discriminated union to capture the various
methods for finding resources (by id, name, or tags).

It also updates several machine api references to note the bifurcated
nature of the api version between v1beta1 and v1.

@openshift-ci openshift-ci bot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Jan 21, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2022

@elmiko: This pull request references Bugzilla bug 2037209, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @jianli-wei

Details

In response to this:

Bug 2037209: update alibaba for provider spec api changes

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/test-infra repository.

@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 21, 2022
@openshift-ci openshift-ci bot requested a review from jianli-wei January 21, 2022 21:04
@elmiko
Copy link
Contributor Author

elmiko commented Jan 21, 2022

this change will require openshift/api#1096 to merge first, it will not build until we can vendor those changes here.
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2022
@elmiko
Copy link
Contributor Author

elmiko commented Jan 21, 2022

this will also need the actuator updated to work properly, see openshift/cluster-api-provider-alibaba#26

@elmiko
Copy link
Contributor Author

elmiko commented Jan 21, 2022

i have added a second commit to vendor from my branch of the api, this is only for testing and should be replaced before we merge.

Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

/approve
These changes look good, but I need to do some locally testing for validation.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2022
@elmiko elmiko force-pushed the update-alibaba-api branch from 76f5532 to 5b2f937 Compare January 21, 2022 21:24
@elmiko
Copy link
Contributor Author

elmiko commented Jan 21, 2022

this will need an update to the vendor to match k8s 1.23, i am coordinating with @staebler to get that change in a separate PR and then we will rebase this PR on top of it.

@elmiko
Copy link
Contributor Author

elmiko commented Jan 21, 2022

i have rebuilt this on top of #5563

@elmiko elmiko force-pushed the update-alibaba-api branch from 6fcda2e to 92f201e Compare January 21, 2022 22:41
@JoelSpeed
Copy link
Contributor

Changes LGTM once we have the API change merged

@elmiko elmiko force-pushed the update-alibaba-api branch 2 times, most recently from 4e8f54c to 6dd72fb Compare January 24, 2022 14:56
@elmiko
Copy link
Contributor Author

elmiko commented Jan 24, 2022

i have revendored for the API changes, we will still need to manually test this one more time and also it will need to wait for #5563

@elmiko elmiko force-pushed the update-alibaba-api branch from 6dd72fb to 394b74e Compare January 24, 2022 18:28
@elmiko
Copy link
Contributor Author

elmiko commented Jan 24, 2022

@elmiko elmiko force-pushed the update-alibaba-api branch from 394b74e to f01e352 Compare January 24, 2022 20:35
This change updates the alibaba provider spec usage related to the
vswitch, security groups, and resource group. The API for the provider
spec is changing to use a discriminated union to capture the various
methods for finding resources (by id, name, or tags).

It also updates several machine api references to note the bifurcated
nature of the api version between v1beta1 and v1.
This change is to update the vendor references to support the Alibaba
resrouce reference updates to the API.
@elmiko elmiko force-pushed the update-alibaba-api branch from f01e352 to 2c8806f Compare January 24, 2022 20:35
@elmiko
Copy link
Contributor Author

elmiko commented Jan 24, 2022

update

@elmiko
Copy link
Contributor Author

elmiko commented Jan 24, 2022

i am removing the hold here since #5563 has merged
/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 24, 2022
@kwoodson
Copy link

I was able to successfully deploy nodes:

test-rx6ff-worker-us-east-1a-vdr6m   Ready    worker   12m   v1.23.0+06791f6
test-rx6ff-worker-us-east-1b-8l777   Ready    worker   12m   v1.23.0+06791f6
test-rx6ff-worker-us-east-1b-plcst   Ready    worker   15m   v1.23.0+06791f6

New fields used:

        resourceGroup:
          id: rg-xxxxxxxx
          type: ID

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2022
@elmiko
Copy link
Contributor Author

elmiko commented Jan 24, 2022

removing the hold as we have all pieces in place and good manual testing
/hold cancel

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 25, 2022

@elmiko: 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/e2e-aws-single-node 2c8806f link false /test e2e-aws-single-node
ci/prow/e2e-ovirt 2c8806f link false /test e2e-ovirt
ci/prow/e2e-aws-workers-rhel7 2c8806f link false /test e2e-aws-workers-rhel7
ci/prow/e2e-crc 2c8806f link false /test e2e-crc
ci/prow/e2e-ibmcloud 2c8806f link false /test e2e-ibmcloud
ci/prow/e2e-azure-upi 2c8806f link false /test e2e-azure-upi
ci/prow/okd-e2e-aws-upgrade 2c8806f link false /test okd-e2e-aws-upgrade
ci/prow/e2e-aws-workers-rhel8 2c8806f link false /test e2e-aws-workers-rhel8

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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 42212b5 into openshift:master Jan 25, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 25, 2022

@elmiko: All pull requests linked via external trackers have merged:

Bugzilla bug 2037209 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2037209: update alibaba for provider spec api changes

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants