Volume name should respect restrictions #47
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pkliczewski
reviewed
Dec 10, 2018
| @@ -52,11 +52,11 @@ def create(args = {}) | |||
| end | |||
|
|
|||
| volumes = [] | |||
|
|
|||
| volume_name = vm_name.split('.')[0] | |||
There was a problem hiding this comment.
Why not to use something we implemented in the apb => https://github.com/ansibleplaybookbundle/import-vm-apb/blob/master/bin/run-v2v.sh#L95 ?
There was a problem hiding this comment.
@pkliczewski changed accordingly to the same logic as in apb
|
@pkliczewski we should wait until ManageIQ/kubeclient#373 is merged. It will determine the actual generated method names for the network attachment definition resource. With the current version of the PR, this PR should be changed (I'll send an updated version soon) |
masayag
force-pushed
the
volume_name_format
branch
from
b0fcd8b to
7bf056f
Compare
In foreman, the VM name may contain also the domain name. When setting this name as the volume name, running the VM fails with the following error: Warning FailedCreate 21s virtualmachine-controller Error creating pod: Pod "virt-launcher-vmi-multus-lqcwz" is invalid: spec.containers[0].name: Invalid value: "volumevmi-multus.example.com": a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?') Therefore the dots from FQDN VM name cannot be part of the volume name. Both dots and underscores will be replaced by dashes.
masayag
force-pushed
the
volume_name_format
branch
from
7bf056f to
17520cf
Compare
pkliczewski
approved these changes
Dec 12, 2018
masayag
added
the
merge to master
masayag
removed
merge to master
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In foreman, the VM name may contain also the domain name.
When setting this name as the volume name, running the VM fails
with the following error:
Therefore the dots from FQDN VM name cannot be part of the volume name.
The patch assumes the name without the domain should be sufficient.
For VM named
stacy-bulgin.example.comthe created volume name isstacy-bulgin-example-com-disk-01