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

Allow to customize VM devices #87

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Allow to customize VM devices #87

merged 1 commit into from
Oct 16, 2020

Conversation

mfranczy
Copy link
Contributor

@mfranczy mfranczy commented Oct 14, 2020 •
edited
Loading

Signed-off-by: Marcin Franczyk [email protected]

How to categorize this PR?

/area performance
/kind enhancement
/priority normal
/platform kubevirt

What this PR does / why we need it:
It allows to customize devices attached to vms

Which issue(s) this PR fixes:
Fixes #36

Special notes for your reviewer:

Release note:

improvement_user

@mfranczy mfranczy requested a review from a team as a code owner October 14, 2020 15:45
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else area/performance Performance (across all domains, such as control plane, networking, storage, etc.) related kind/enhancement Enhancement, improvement, extension platform/kubevirt Container Native Virtualization (CNV) KubeVirt platform/infrastructure priority/normal needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Oct 14, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 14, 2020
@mfranczy
Copy link
Contributor Author

mfranczy commented Oct 14, 2020 •
edited
Loading

@stoyanr after a several attempts, I decided to take a bit different approach then we spoke, I will explain it tomorrow during sync meeting, the PR is still WIP, I have to add doc and validation. Speaking about validation we will need to change it a bit, more details tomorrow during the sync meeting.

@gardener-robot gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) and removed size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot gardener-robot added the size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) label Oct 15, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot gardener-robot removed the size/l Size of pull request is large (see gardener-robot robot/bots/size.py) label Oct 15, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@mfranczy mfranczy changed the title [WIP] Allow to customize VM devices Allow to customize VM devices Oct 15, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 15, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
stoyanr
stoyanr suggested changes Oct 15, 2020
View reviewed changes
pkg/apis/kubevirt/validation/worker.go Outdated Show resolved Hide resolved
pkg/apis/kubevirt/validation/worker.go Outdated Show resolved Hide resolved
pkg/admission/validator/shoot.go Show resolved Hide resolved
pkg/admission/validator/shoot.go Show resolved Hide resolved
pkg/admission/validator/shoot.go Show resolved Hide resolved
pkg/apis/kubevirt/validation/worker.go Show resolved Hide resolved
pkg/apis/kubevirt/validation/worker_test.go Outdated Show resolved Hide resolved
pkg/apis/kubevirt/validation/worker_test.go Outdated Show resolved Hide resolved
pkg/controller/worker/machines_test.go Outdated Show resolved Hide resolved
pkg/apis/kubevirt/validation/worker.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added needs/changes Needs (more) changes and removed needs/review Needs review labels Oct 15, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 15, 2020
@mfranczy
Copy link
Contributor Author

mfranczy commented Oct 15, 2020 •
edited
Loading

@stoyanr I fixed the PR according to your comments, speaking about passing the root volume, I think it's not needed as described here: #87 (comment)

root volume won't be validated there, I have to consider appropriate use cases when user defines root-disk, like here:
https://github.com/gardener/gardener-extension-provider-kubevirt/pull/87/files#diff-c5505e315935886d70ddbe28d4e6e6290b2396305bca954ee55c4f3515ae6641R61-R77

@mfranczy
allow to customize VM devices
f105e6a 
Signed-off-by: Marcin Franczyk <[email protected]>
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 15, 2020
@stoyanr stoyanr merged commit 475c468 into gardener-attic:master Oct 16, 2020
@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stoyanr stoyanr stoyanr approved these changes

Assignees
No one assigned
Labels
area/performance Performance (across all domains, such as control plane, networking, storage, etc.) related kind/api-change API change with impact on API users kind/enhancement Enhancement, improvement, extension needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/second-opinion Needs second review by someone else platform/kubevirt Container Native Virtualization (CNV) KubeVirt platform/infrastructure size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add additional VM configuration options
6 participants
@mfranczy @stoyanr @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot