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.

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

Bug 1891543: openstack: consider volumes for storage requirements checks #4323

Merged
merged 1 commit into from Nov 4, 2020

Conversation

EmilienM
Copy link
Member

@EmilienM EmilienM commented Oct 29, 2020

It is possible to boot a server without using ephemeral storage, with
boot from volume:
https://docs.openstack.org/cinder/latest/admin/blockstorage-boot-from-volume.html

This patch will disable flavor storage check if rootVolume is used in
the Machine Pool and checks that the volume is at least 25Gb sized.

https://bugzilla.redhat.com/show_bug.cgi?id=1891543

@EmilienM
Copy link
Member Author

/label platform/openstack

@EmilienM
Copy link
Member Author

/assign @EmilienM

@EmilienM
Copy link
Member Author

/cc pierreprinetti

@EmilienM
Copy link
Member Author

/test e2e-crc

@EmilienM EmilienM changed the title openstack: allow a flavor to use ephemeral storage Bug 1891543: openstack: allow a flavor to use ephemeral storage Oct 29, 2020
@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. label Oct 29, 2020
@openshift-ci-robot
Copy link
Contributor

@EmilienM: This pull request references Bugzilla bug 1891543, which is invalid:

  • expected the bug to target the "4.7.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1891543: openstack: allow a flavor to use ephemeral storage

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/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Oct 29, 2020
@EmilienM
Copy link
Member Author

/bugzilla refresh

@openshift-ci-robot
Copy link
Contributor

@EmilienM: This pull request references Bugzilla bug 1891543, which is invalid:

  • expected the bug to target the "4.7.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

@EmilienM
Copy link
Member Author

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Oct 29, 2020
@openshift-ci-robot
Copy link
Contributor

@EmilienM: This pull request references Bugzilla bug 1891543, which is valid. The bug has been moved to the POST state.

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

In response to this:

/bugzilla refresh

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.

Copy link

@adduarte adduarte left a comment

Choose a reason for hiding this comment

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

just a couple of comments

Copy link
Member

@pierreprinetti pierreprinetti left a comment

Choose a reason for hiding this comment

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

@EmilienM
I think that you are not addressing the problem reported in the bug.

To my understanding, the bug report says:


Do not check the flavor disk size when I set OCP nodes to boot from volume. When I pass a volume in an install-config.yaml machine-pool, the disk capacity of the flavor is irrelevant, and the volume size should be checked instead.


The interesting part though, is that you are addressing the case of the ephemeral disks which we haven't considered so far. This might be worth some investigation; however, I suspect that we would need to write some new code to be able to automatically use ephemeral storage (as in "OS-FLV-EXT-DATA:ephemeral") for our purposes (ie: is it automatically mounted? To which mountpoint?).

@ppeereb1
Copy link

I'm also running into this when bootstrapping a new 4.6.1 cluster. Unfortunately our cloud has both ephemeral and root disk size to 0.

paul@paul-ThinkPad-X1-Carbon-6th:~$ openstack flavor show m1.4large
+----------------------------+--------------------------------------+
| Field                      | Value                                |
+----------------------------+--------------------------------------+
| OS-FLV-DISABLED:disabled   | False                                |
| OS-FLV-EXT-DATA:ephemeral  | 0                                    |
| access_project_ids         | None                                 |
| disk                       | 0                                    |
| id                         | 6d14ea74-f17d-43d2-84a9-fbdfa12561a4 |
| name                       | m1.4large                            |
| os-flavor-access:is_public | True                                 |
| properties                 | ostype='generic'                     |
| ram                        | 16384                                |
| rxtx_factor                | 1.0                                  |
| swap                       |                                      |
| vcpus                      | 4                                    |
+----------------------------+--------------------------------------+

They are expecting you're using a cinder volume for booting a vm. You can create a separate root volume when you specify the rootVolume parameter in install-config.yaml: https://github.com/openshift/installer/blob/master/docs/user/openstack/customization.md#custom-machine-pools

It would be nice if the disk check is dropped when specifying a rootVolume parameter in the install-config.yaml

@EmilienM
Copy link
Member Author

EmilienM commented Oct 29, 2020

[...]

They are expecting you're using a cinder volume for booting a vm. You can create a separate root volume when you specify the rootVolume parameter in install-config.yaml: https://github.com/openshift/installer/blob/master/docs/user/openstack/customization.md#custom-machine-pools

It would be nice if the disk check is dropped when specifying a rootVolume parameter in the install-config.yaml

Ok, I see what the problem is now. Thanks for taking the time to explain. I'll add a check for the rootVolume.size value.

@pierreprinetti
Copy link
Member

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels Oct 29, 2020
@openshift-ci-robot
Copy link
Contributor

@pierreprinetti: This pull request references Bugzilla bug 1891543, which is valid.

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

In response to this:

/bugzilla refresh

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-bot
Copy link
Contributor

/retest

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

13 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-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

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

Test name Commit Details Rerun command
ci/prow/e2e-ovirt 1b400b4 link /test e2e-ovirt
ci/prow/e2e-crc 1b400b4 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 6e09ad3 into openshift:master Nov 4, 2020
@openshift-ci-robot
Copy link
Contributor

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

Bugzilla bug 1891543 has been moved to the MODIFIED state.

In response to this:

Bug 1891543: openstack: consider volumes for storage requirements checks

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.

@EmilienM EmilienM deleted the bz/1891543 branch November 4, 2020 15:15
@EmilienM
Copy link
Member Author

EmilienM commented Nov 5, 2020

/cherrypick release-4.6

@openshift-cherrypick-robot

@EmilienM: #4323 failed to apply on top of branch "release-4.6":

Applying: openstack: consider volumes for storage requirements checks
Using index info to reconstruct a base tree...
M	docs/user/openstack/README.md
M	docs/user/openstack/customization.md
M	pkg/asset/installconfig/openstack/validation/machinepool.go
M	pkg/asset/installconfig/openstack/validation/machinepool_test.go
M	pkg/asset/installconfig/openstack/validation/platform.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/asset/installconfig/openstack/validation/platform.go
CONFLICT (content): Merge conflict in pkg/asset/installconfig/openstack/validation/platform.go
Auto-merging pkg/asset/installconfig/openstack/validation/machinepool_test.go
Auto-merging pkg/asset/installconfig/openstack/validation/machinepool.go
CONFLICT (content): Merge conflict in pkg/asset/installconfig/openstack/validation/machinepool.go
Auto-merging docs/user/openstack/customization.md
Auto-merging docs/user/openstack/README.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 openstack: consider volumes for storage requirements checks
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-4.6

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.

@EmilienM EmilienM restored the bz/1891543 branch November 5, 2020 15:16
@EmilienM EmilienM deleted the bz/1891543 branch November 5, 2020 15:16
@EmilienM EmilienM restored the bz/1891543 branch November 5, 2020 15:21
@EmilienM
Copy link
Member Author

EmilienM commented Nov 5, 2020

We need #4344 to land before backporting this one

@EmilienM
Copy link
Member Author

/cherrypick release-4.6

@openshift-cherrypick-robot

@EmilienM: #4323 failed to apply on top of branch "release-4.6":

Applying: openstack: consider volumes for storage requirements checks
Using index info to reconstruct a base tree...
M	docs/user/openstack/README.md
M	docs/user/openstack/customization.md
M	pkg/asset/installconfig/openstack/validation/machinepool.go
M	pkg/asset/installconfig/openstack/validation/machinepool_test.go
M	pkg/asset/installconfig/openstack/validation/platform.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/asset/installconfig/openstack/validation/platform.go
CONFLICT (content): Merge conflict in pkg/asset/installconfig/openstack/validation/platform.go
Auto-merging pkg/asset/installconfig/openstack/validation/machinepool_test.go
Auto-merging pkg/asset/installconfig/openstack/validation/machinepool.go
CONFLICT (content): Merge conflict in pkg/asset/installconfig/openstack/validation/machinepool.go
Auto-merging docs/user/openstack/customization.md
Auto-merging docs/user/openstack/README.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 openstack: consider volumes for storage requirements checks
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-4.6

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-medium Referenced Bugzilla bug's severity is medium 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. platform/openstack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet