Skip to content

Bug 1820434: OpenStack: Allow Booting bootstrap node from volume#3434

Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
mandre:bootstrap-bfv
Apr 18, 2020
Merged

Bug 1820434: OpenStack: Allow Booting bootstrap node from volume#3434
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
mandre:bootstrap-bfv

Conversation

@mandre
Copy link
Member

@mandre mandre commented Apr 9, 2020

Some OpenStack environments might enforce a policy where VMs can't boot
with ephemeral disks and need to boot from a cinder volume. In these
cases, it would be impossible to install OCP because even though we
configured the master and worker nodes to boot from volume using the
rootVolume setting, the bootstrap node would still boot from
ephemeral disk.

With this change, the bootstrap node follows the rootVolume settings
from controlPlane machine pool. This shouldn't be a surprising behavior
since the bootstrap node is already using the same flavor and security
groups as the control plane nodes.

@openshift-ci-robot
Copy link
Contributor

@mandre: This pull request references Bugzilla bug 1820434, which is valid. The bug has been moved to the POST state. 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.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1820434: OpenStack: Allow Booting bootstrap node from volume

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-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Apr 9, 2020
@pierreprinetti
Copy link
Member

/approve
/label platform/openstack

@Fedosin
Copy link
Contributor

Fedosin commented Apr 9, 2020

/lgtm
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 9, 2020
@iamemilio
Copy link

iamemilio commented Apr 9, 2020

/hold
I think it might be confusing that whether or not bootstrap nodes boot from volume is determined by a setting on the master machine pool. Also, if this is the route we choose to go, the docs should clarify that adding a boot from volume to the master machine pool, also boots the bootstrap from volume.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2020
@mandre
Copy link
Member Author

mandre commented Apr 9, 2020

/hold
I think it might be confusing that whether or not bootstrap nodes boot from volume is determined by a setting on the master machine pool. Also, needs docs.

I do not think we need docs around this. The boostrap node is ephemeral and we don't really care how it's provisioned or how it works, as long as it works - it's a black box. This patch makes it work in environment where you're not allowed to provision VMs with ephemeral disks, by following the settings from the control plane machine pool. I believe this is the expected behavior since we're already using the flavor and security groups from the control plane for the bootstrap node.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2020
@mandre mandre force-pushed the bootstrap-bfv branch 2 times, most recently from 4605be5 to 8471bd8 Compare April 10, 2020 09:43
@mandre
Copy link
Member Author

mandre commented Apr 10, 2020

/test e2e-openstack

1 similar comment
@mandre
Copy link
Member Author

mandre commented Apr 11, 2020

/test e2e-openstack

mandre added 2 commits April 14, 2020 09:58
Some OpenStack environments might enforce a policy where VMs can't boot
with ephemeral disks and need to boot from a cinder volume. In these
cases, it would be impossible to install OCP because even though we
configured the master and worker nodes to boot from volume using the
`rootVolume` setting, the bootstrap node would still boot from
ephemeral disk.

With this change, the bootstrap node follows the `rootVolume` settings
from controlPlane machine pool. This shouldn't be a surprising behavior
since the bootstrap node is already using the same flavor and security
groups as the control plane nodes.
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierreprinetti

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

@pierreprinetti
Copy link
Member

/lgtm
/retest

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

/test e2e-openstack

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Apr 14, 2020

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

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 1f589d4 link /test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@Fedosin
Copy link
Contributor

Fedosin commented Apr 18, 2020

/test e2e-openstack

@Fedosin
Copy link
Contributor

Fedosin commented Apr 18, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 18, 2020
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit ab42046 into openshift:master Apr 18, 2020
@openshift-ci-robot
Copy link
Contributor

@mandre: All pull requests linked via external trackers have merged: openshift/installer#3434. Bugzilla bug 1820434 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1820434: OpenStack: Allow Booting bootstrap node from volume

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

/cherry-pick release-4.4

@openshift-cherrypick-robot

@pierreprinetti: #3434 failed to apply on top of branch "release-4.4":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	data/data/openstack/bootstrap/main.tf
M	data/data/openstack/bootstrap/variables.tf
M	data/data/openstack/main.tf
Falling back to patching base and 3-way merge...
Auto-merging data/data/openstack/main.tf
CONFLICT (content): Merge conflict in data/data/openstack/main.tf
Auto-merging data/data/openstack/bootstrap/variables.tf
CONFLICT (content): Merge conflict in data/data/openstack/bootstrap/variables.tf
Auto-merging data/data/openstack/bootstrap/main.tf
Patch failed at 0001 Bug 1820434: OpenStack: Allow Booting bootstrap node from volume

Details

In response to this:

/cherry-pick release-4.4

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/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. platform/openstack

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants