Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Openstack primary subnet using machine spec #4346

Merged
merged 5 commits into from
Dec 7, 2020
Merged

Openstack primary subnet using machine spec #4346

merged 5 commits into from
Dec 7, 2020

Conversation

iamemilio
Copy link

@iamemilio iamemilio commented Nov 4, 2020 •
edited by openshift-ci-robot
Loading

Sets primary subnet for OpenStack in the provider spec

Users with more restrictive permissions, and especially those using provider networks, may not be able to set tags on their network components. This change makes it so that we no longer make any modifications to user provided networks and subnets.

Fixes: OSASINFRA-2145

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 4, 2020
@staebler
Copy link
Contributor

staebler commented Nov 4, 2020

Please follow contributing guidelines for the commit message.

@iamemilio
Copy link
Author

/retest

@iamemilio iamemilio changed the title WIP: Openstack primary subnet using machine spec Openstack primary subnet using machine spec Nov 6, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2020
@mandre
Copy link
Member

mandre commented Nov 10, 2020

/label platform/openstack

mandre
mandre approved these changes Nov 10, 2020
View reviewed changes
Copy link
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2020
@pierreprinetti
Copy link
Member

/test e2e-openstack-ipi-ipi

@openshift-ci-robot
Copy link
Contributor

@pierreprinetti: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

  • /test e2e-aws
  • /test e2e-aws-disruptive
  • /test e2e-aws-fips
  • /test e2e-aws-proxy
  • /test e2e-aws-rhel8
  • /test e2e-aws-shared-vpc
  • /test e2e-aws-upgrade
  • /test e2e-aws-upi
  • /test e2e-aws-workers-rhel7
  • /test e2e-azure
  • /test e2e-azure-shared-vpc
  • /test e2e-azure-upi
  • /test e2e-crc
  • /test e2e-gcp
  • /test e2e-gcp-shared-vpc
  • /test e2e-gcp-upgrade
  • /test e2e-gcp-upi
  • /test e2e-gcp-upi-xpn
  • /test e2e-libvirt
  • /test e2e-metal
  • /test e2e-metal-assisted
  • /test e2e-metal-ipi
  • /test e2e-openstack
  • /test e2e-openstack-ipi
  • /test e2e-openstack-parallel
  • /test e2e-openstack-upi
  • /test e2e-ovirt
  • /test e2e-vsphere
  • /test e2e-vsphere-upi
  • /test gofmt
  • /test golint
  • /test govet
  • /test images
  • /test shellcheck
  • /test tf-fmt
  • /test tf-lint
  • /test unit
  • /test verify-codegen
  • /test verify-vendor
  • /test yaml-lint

Use /test all to run the following jobs:

  • pull-ci-openshift-installer-master-e2e-aws
  • pull-ci-openshift-installer-master-e2e-aws-fips
  • pull-ci-openshift-installer-master-e2e-aws-upgrade
  • pull-ci-openshift-installer-master-e2e-aws-workers-rhel7
  • pull-ci-openshift-installer-master-e2e-crc
  • pull-ci-openshift-installer-master-e2e-libvirt
  • pull-ci-openshift-installer-master-e2e-metal-ipi
  • pull-ci-openshift-installer-master-e2e-openstack
  • 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-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

In response to this:

/test e2e-openstack-ipi-ipi

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.

@pierreprinetti
Copy link
Member

Sorry for the noise, please ignore me. I want to test the step-registry suite

/test e2e-openstack-ipi

@pierreprinetti
Copy link
Member

@iamemilio Next time, it would be very handy if you could link the corresponding change in the consumer (CAPO, I suppose), in the commit message or in the PR description :)

@mtnbikenc
Copy link
Member

/uncc

@openshift-ci-robot openshift-ci-robot removed the request for review from mtnbikenc November 11, 2020 16:24
@pierreprinetti
Copy link
Member

/test e2e-openstack-ipi
(note that the actual e2e-openstack test has succeeded already 🟢 )

mandre
mandre reviewed Nov 13, 2020
View reviewed changes
Copy link
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

/lgtm cancel

pkg/tfvars/openstack/openstack.go Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 13, 2020
@iamemilio iamemilio force-pushed the openstack-primary-subnet-using-machine-spec branch 2 times, most recently from ebbbd32 to 7fa7e0b Compare November 16, 2020 16:26
Fedosin
Fedosin reviewed Nov 16, 2020
View reviewed changes
Copy link
Contributor

@Fedosin Fedosin left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020
@iamemilio
Copy link
Author

/test e2e-openstack

@iamemilio
Copy link
Author

/retest

@iamemilio
Copy link
Author

/lgtm

@openshift-ci-robot
Copy link
Contributor

@iamemilio: you cannot LGTM your own PR.

In response to this:

/lgtm

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.

@iamemilio
Copy link
Author

boo 👎

@iamemilio iamemilio mentioned this pull request Dec 6, 2020
Closed
staebler and others added 5 commits December 6, 2020 17:09
@staebler
vendor: use local stripped down copy of cluster-api-provider-kubevirt
20567f7 
The cluster-api-provider-kubevirt module is dependent upon the
bitbucket.org/ww/goautoneg, which no longer exists. **Temporarily**,
copy the files needed from cluster-api-provider-kubevirt into
the repo and replace the module with a reference to those files.
@michaelgugino
Update MAO depedencies
37d3621 
This commit updates various MAO dependencies.
@michaelgugino
Update cluster-api references to mao
b0b9646 
openshift/cluster-api project is deprecated in favor
of openshift/machine-api-operator.  This commit
updates references and fixes incorrect references to
v1beta.ObjectMeta types.
@staebler
vendor: go back to using v0.3.1 for controller-tools
a77334d 
With v0.4.0, controller-gen no longer accepts the IPNet type
when generating the CRD for the install config.
Set OpenStack primarySubnet in MachineSpec
42098a0 
In the OpenStack platform, the primary subnet is where a node's kubernetes endpoints are published.
Right now, we set this with a tag in OpenStack. Due to some users having restrictions on tagging
resources, and this being a difficult user interface, we added a way to modify this value in the
machine api. We still fully support setting this value with tagging for backwards compatibility
reasons. In this patch, we update the installer code to set the primarySubnet using the MachineSpec.
@iamemilio iamemilio force-pushed the openstack-primary-subnet-using-machine-spec branch from 6cee4ea to 42098a0 Compare December 6, 2020 22:24
@adduarte
Copy link

adduarte commented Dec 6, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2020
@openshift-bot
Copy link
Contributor

/retest

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

6 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@iamemilio
Copy link
Author

/retest

@openshift-bot
Copy link
Contributor

/retest

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

7 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Dec 7, 2020 •
edited
Loading

@iamemilio: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-openstack-ipi b81f496 link /test e2e-openstack-ipi
ci/prow/e2e-openstack 42098a0 link /test e2e-openstack
ci/prow/e2e-ovirt 42098a0 link /test e2e-ovirt
ci/prow/e2e-crc 42098a0 link /test e2e-crc

Full PR test history. Your PR dashboard.

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 3c28bc3 into openshift:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mandre mandre mandre approved these changes

@MaysaMacedo MaysaMacedo MaysaMacedo left review comments

@Fedosin Fedosin Fedosin left review comments

@staebler staebler staebler left review comments

@EmilienM EmilienM EmilienM approved these changes

Assignees

@mandre mandre

@EmilienM EmilienM

@Fedosin Fedosin

@staebler staebler

@adduarte adduarte

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. platform/openstack
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.