Add IBM Power VS: types by clnperez · Pull Request #5609 · openshift/installer

clnperez · 2022-02-04T22:52:27Z

For more background on IPI on Power VS, refer to the enhancement
proposal here: openshift/enhancements#736

Older discussions on some of the code here can be found in
#5224.
An older unreviewed version of this PR can be found here:
#5292

Signed-off-by: Christy Norman christy@linux.vnet.ibm.com

clnperez · 2022-02-04T23:10:07Z

@rna-afk can you advise on how we should add the right reviewers and owners to some of these files? Back in October the answer was to add what the checks are complaining about, so sounds like we need to be educated on the new best practices there.

staebler · 2022-02-05T01:09:14Z

The text here implies to me that you can have a successful install without specifying the VPC, in which case your cluster will not use VPC Load Balancers. Is that accurate? Or is the optional aspect of this like it is for other platforms where omitting the VPC tells the installer to create a VPC for you?

ah -- i blanked on explaining that a few things here are temporary. we've had to pause on including the vpc creation, so for now there are manual steps for dev that we need to take to create a vpc and the networks in both ibmcloud and power vs. after that, vpc will be created by the installer. it's not going to be optional when we've got networking a fix into prod.

Let's still clean up the text in the description to represent the current behavior.

i did clean some things up a couple of weeks ago, but, sounds like youre asking for something additional. can you elaborate on where you want current behavior documented?

staebler · 2022-02-05T01:31:24Z

@rna-afk can you advise on how we should add the right reviewers and owners to some of these files? Back in October the answer was to add what the checks are complaining about, so sounds like we need to be educated on the new best practices there.

Add the following to all your installer PRs. Aditya will be the point for doing reviews.
/cc @rna-afk

rna-afk · 2022-02-07T13:52:21Z

For the owners file, this looks right but you should add the alias (multiarch-reviewers/approvers) in this file. Also you need to add the former in almost every powervs folder you create in the installer. I can guide you on which folders.

kirankt · 2022-02-09T14:01:58Z

For the owners file, this looks right but you should add the alias (multiarch-reviewers/approvers) in this file. Also you need to add the former in almost every powervs folder you create in the installer. I can guide you on which folders.

Question. Why multiarch-{reviewers,approvers} as opposed to powervs-{reviewers,approvers}? Looks like the file follows platform-{reviewers,approvers} format.

rna-afk · 2022-02-09T14:09:02Z

Question. Why multiarch-{reviewers,approvers} as opposed to powervs-{reviewers,approvers}? Looks like the file follows platform-{reviewers,approvers} format.

Not sure if there's a preference on the naming and well now I think powervs would be better. Thanks!

clnperez · 2022-02-09T15:08:50Z

multiarch-reviewers comes from the name of the team I work closely with inside Red Hat. @Prashanth684 is the dev I work the most with. Thoughts, Prashanth?

clnperez · 2022-02-09T15:17:48Z

I think I've addressed all @staebler's comments and fixed the two checks that were failing. The only outstanding questions may be around some of our temporary dev networking requirements that we should be able to remove in a week or so. Hopefully everyone is okay with a followup PR for those.

Prashanth684 · 2022-02-09T15:42:38Z

multiarch-reviewers comes from the name of the team I work closely with inside Red Hat. @Prashanth684 is the dev I work the most with. Thoughts, Prashanth?

i would add myself, @r4f4 and @AnnaZivkovic as they work on the installer side of things.and maybe some folks from IBM as well who worked on this

rna-afk

Looks good. Just a few nits/questions

clnperez · 2022-02-09T20:41:27Z

after a quick chat with @Prashanth684 we decided to switch to powervs-[reviewers|approvers] vs multiarch, and not add the other two IDs he listed.

hopefully good to go now. i've looked through failing tests on each run (with a special focus on the ibmcloud ones, since we share some things -- though nothing in this PR so just being extra careful) and nothing looks related.

clnperez · 2022-02-09T21:34:10Z

/retest ci/prow/unit
/retest ci/prow/okd-unit

openshift-ci · 2022-02-09T21:34:11Z

@clnperez: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test e2e-aws
/test e2e-aws-upgrade
/test e2e-gcp-upgrade
/test e2e-metal-ipi-ovn-ipv6-required
/test gofmt
/test golint
/test govet
/test images
/test okd-images
/test okd-unit
/test okd-verify-codegen
/test openstack-manifests
/test shellcheck
/test tf-lint
/test unit
/test verify-codegen
/test verify-vendor
/test yaml-lint

The following commands are available to trigger optional jobs:

/test e2e-alibaba
/test e2e-aws-disruptive
/test e2e-aws-fips
/test e2e-aws-proxy
/test e2e-aws-rhel8
/test e2e-aws-shared-vpc
/test e2e-aws-single-node
/test e2e-aws-upi
/test e2e-aws-upi-proxy
/test e2e-aws-workers-rhel8
/test e2e-azure
/test e2e-azure-resourcegroup
/test e2e-azure-shared-vpc
/test e2e-azure-upi
/test e2e-azurestack
/test e2e-azurestack-upi
/test e2e-crc
/test e2e-gcp
/test e2e-gcp-shared-vpc
/test e2e-gcp-upi
/test e2e-gcp-upi-xpn
/test e2e-ibmcloud
/test e2e-libvirt
/test e2e-metal
/test e2e-metal-assisted
/test e2e-metal-ipi
/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-swapped-hosts
/test e2e-metal-ipi-virtualmedia
/test e2e-metal-single-node-live-iso
/test e2e-openstack
/test e2e-openstack-kuryr
/test e2e-openstack-parallel
/test e2e-openstack-proxy
/test e2e-openstack-upi
/test e2e-ovirt
/test e2e-vsphere
/test e2e-vsphere-upi
/test okd-e2e-aws
/test okd-e2e-aws-upgrade
/test okd-e2e-gcp
/test okd-e2e-gcp-upgrade
/test okd-e2e-vsphere
/test tf-fmt

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-installer-master-e2e-aws
pull-ci-openshift-installer-master-e2e-aws-fips
pull-ci-openshift-installer-master-e2e-aws-single-node
pull-ci-openshift-installer-master-e2e-aws-upgrade
pull-ci-openshift-installer-master-e2e-aws-workers-rhel8
pull-ci-openshift-installer-master-e2e-azure-upi
pull-ci-openshift-installer-master-e2e-crc
pull-ci-openshift-installer-master-e2e-ibmcloud
pull-ci-openshift-installer-master-e2e-libvirt
pull-ci-openshift-installer-master-e2e-metal-ipi-ovn-ipv6
pull-ci-openshift-installer-master-e2e-metal-single-node-live-iso
pull-ci-openshift-installer-master-e2e-openstack
pull-ci-openshift-installer-master-e2e-openstack-kuryr
pull-ci-openshift-installer-master-e2e-ovirt
pull-ci-openshift-installer-master-gofmt
pull-ci-openshift-installer-master-golint
pull-ci-openshift-installer-master-govet
pull-ci-openshift-installer-master-images
pull-ci-openshift-installer-master-okd-e2e-aws
pull-ci-openshift-installer-master-okd-e2e-aws-upgrade
pull-ci-openshift-installer-master-okd-images
pull-ci-openshift-installer-master-okd-unit
pull-ci-openshift-installer-master-okd-verify-codegen
pull-ci-openshift-installer-master-openstack-manifests
pull-ci-openshift-installer-master-shellcheck
pull-ci-openshift-installer-master-tf-fmt
pull-ci-openshift-installer-master-tf-lint
pull-ci-openshift-installer-master-unit
pull-ci-openshift-installer-master-verify-codegen
pull-ci-openshift-installer-master-verify-vendor
pull-ci-openshift-installer-master-yaml-lint

Details

In response to this:

/retest ci/prow/unit
/retest ci/prow/okd-unit

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.

clnperez · 2022-02-09T21:34:55Z

/test unit
/test okd-unit

openshift-bot · 2022-02-25T08:37:16Z

/retest-required

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

openshift-bot · 2022-02-25T09:29:19Z

/retest-required

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

openshift-bot · 2022-02-25T10:21:20Z

/retest-required

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

openshift-bot · 2022-02-25T10:34:32Z

/retest-required

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

openshift-bot · 2022-02-25T11:23:18Z

/retest-required

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

openshift-bot · 2022-02-25T12:11:23Z

/retest-required

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

openshift-bot · 2022-02-25T13:35:21Z

/retest-required

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

openshift-bot · 2022-02-25T13:47:19Z

/retest-required

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

openshift-bot · 2022-02-25T14:35:20Z

/retest-required

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

staebler · 2022-02-25T15:03:48Z

/lgtm cancel
The unit and verify-codegen pre-submit checks are failing.

For more background on IPI on Power VS, refer to the enhancement proposal here: openshift/enhancements#736 Older discussions on some of the code here can be found in openshift#5224. An older unreviewed version of this PR can be found here: openshift#5292 Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

clnperez · 2022-02-26T17:18:35Z

sorry missed a typo. fixed and those are passing now. PTAL @staebler

rna-afk · 2022-02-28T13:36:01Z

/lgtm

openshift-bot · 2022-02-28T13:58:13Z

/retest-required

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

openshift-bot · 2022-02-28T15:12:17Z

/retest-required

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

openshift-bot · 2022-02-28T15:51:14Z

/retest-required

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

openshift-bot · 2022-02-28T16:17:14Z

/retest-required

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

openshift-bot · 2022-02-28T16:56:14Z

/retest-required

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

openshift-bot · 2022-02-28T17:48:23Z

/retest-required

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

openshift-bot · 2022-02-28T18:40:06Z

/retest-required

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

openshift-ci · 2022-02-28T19:10:20Z

@clnperez: 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-libvirt	`1604905`	link	false	`/test e2e-libvirt`
ci/prow/okd-e2e-aws	`1604905`	link	false	`/test okd-e2e-aws`
ci/prow/e2e-azure-upi	`1604905`	link	false	`/test e2e-azure-upi`
ci/prow/okd-e2e-aws-upgrade	`1604905`	link	false	`/test okd-e2e-aws-upgrade`
ci/prow/e2e-openstack	`1604905`	link	false	`/test e2e-openstack`
ci/prow/e2e-metal-single-node-live-iso	`1604905`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/e2e-ibmcloud	`1604905`	link	false	`/test e2e-ibmcloud`
ci/prow/e2e-crc	`1604905`	link	false	`/test e2e-crc`
ci/prow/e2e-aws-workers-rhel8	`1604905`	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.

clnperez · 2022-02-28T20:47:19Z

/test e2e-aws
this one's being weird. timeouts and image pull issues. @rna-afk any idea if something's up with ci?

openshift-bot · 2022-02-28T21:02:53Z

/retest-required

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

rna-afk · 2022-02-28T21:56:21Z

Not aware of any problems in CI right now. The e2e-aws test is failing since this morning. Probably some kind of performance issue with AWS.

openshift-ci Bot requested review from jhixson74 and kirankt February 4, 2022 22:53

openshift-ci Bot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Feb 4, 2022

clnperez changed the title ~~Add IBM Power VS: types only~~ Add IBM Power VS: types Feb 4, 2022

staebler reviewed Feb 5, 2022

View reviewed changes

openshift-ci Bot requested a review from rna-afk February 5, 2022 01:31

clnperez force-pushed the new-upstream-types branch from 69521ee to 5586d35 Compare February 9, 2022 03:45

openshift-ci Bot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Feb 9, 2022

clnperez force-pushed the new-upstream-types branch from 5586d35 to 8550dad Compare February 9, 2022 03:53

rna-afk reviewed Feb 9, 2022

View reviewed changes

Comment thread data/data/install.openshift.io_installconfigs.yaml Outdated

rna-afk reviewed Feb 9, 2022

View reviewed changes

Comment thread pkg/types/powervs/validation/platform.go Outdated

Comment thread pkg/types/powervs/validation/machinepool.go Outdated

Comment thread pkg/types/powervs/validation/machinepool.go Outdated

Comment thread pkg/types/powervs/validation/machinepool.go Outdated

Prashanth684 reviewed Feb 9, 2022

View reviewed changes

Comment thread pkg/types/powervs/validation/machinepool.go Outdated

clnperez force-pushed the new-upstream-types branch from 8550dad to 780de9a Compare February 9, 2022 20:33

mjturek reviewed Feb 9, 2022

View reviewed changes

Comment thread pkg/types/powervs/validation/machinepool_test.go Outdated

clnperez force-pushed the new-upstream-types branch from 780de9a to 602dbc8 Compare February 9, 2022 21:44

clnperez mentioned this pull request Feb 9, 2022

Add IBM Power VS: manifest assets #5610

Merged

Prashanth684 reviewed Feb 9, 2022

View reviewed changes

Comment thread pkg/types/powervs/powervs_regions.go Outdated

Prashanth684 reviewed Feb 9, 2022

View reviewed changes

Comment thread data/data/install.openshift.io_installconfigs.yaml Outdated

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2022

clnperez force-pushed the new-upstream-types branch from fc2c342 to 1604905 Compare February 26, 2022 17:06

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2022

openshift-merge-robot merged commit 4f774da into openshift:master Feb 28, 2022

clnperez mentioned this pull request Mar 14, 2022

Add IBM Power VS: cluster assets #5651

Merged

bkhadars mentioned this pull request Mar 24, 2022

Add VolumeID testscases for Power VS machinepool #5742

Merged

Conversation

clnperez commented Feb 4, 2022

Uh oh!

clnperez commented Feb 4, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staebler Feb 5, 2022

Choose a reason for hiding this comment

Uh oh!

clnperez Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

staebler Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

clnperez Feb 22, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staebler commented Feb 5, 2022

Uh oh!

rna-afk commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirankt commented Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rna-afk commented Feb 9, 2022

Uh oh!

clnperez commented Feb 9, 2022

Uh oh!

clnperez commented Feb 9, 2022

Uh oh!

Uh oh!

Prashanth684 commented Feb 9, 2022

Uh oh!

rna-afk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clnperez commented Feb 9, 2022

Uh oh!

Uh oh!

clnperez commented Feb 9, 2022

Uh oh!

openshift-ci Bot commented Feb 9, 2022

Uh oh!

clnperez commented Feb 9, 2022

Uh oh!

Uh oh!

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

openshift-bot commented Feb 25, 2022

Uh oh!

rna-afk commented Feb 7, 2022 •

edited

Loading

kirankt commented Feb 9, 2022 •

edited

Loading

openshift-ci Bot commented Feb 28, 2022 •

edited

Loading