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

Load balancer selector changes #143

Merged
merged 4 commits into from
Sep 5, 2022
Merged

Conversation

nirarg
Copy link
Contributor

@nirarg nirarg commented Sep 2, 2022

Change LoadBalancer use existing labels on VMI instead of setting new

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Sep 2, 2022
@nirarg
Copy link
Contributor Author

nirarg commented Sep 2, 2022

@mfranczy @qinqon @davidvossel
I opened the PR to start have review on the logic changes
The PR is still missing unit tests and comments fixes

@nirarg nirarg changed the title Change LoadBalancer use existing labels on VMI instead of setting new [WIP] Load balancer selector changes Sep 2, 2022
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2022
@qinqon
Copy link
Contributor

qinqon commented Sep 2, 2022

Let's fix the CI issue first before review, does not make sense to review non working code.

@mfranczy
Copy link
Contributor

mfranczy commented Sep 5, 2022

@nirarg could you as well add a short note to documentation about labels that should be added to VMs?

* TestEnsureLoadBalancer
* TestUpdateLoadBalancer
* TestEnsureLoadBalancerDeleted

Signed-off-by: Nir Argaman <[email protected]>
@nirarg nirarg changed the title [WIP] Load balancer selector changes Load balancer selector changes Sep 5, 2022
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2022
Describe about the the Labels must be used during the tenant cluster nodes creation

Signed-off-by: Nir Argaman <[email protected]>
@nirarg
Copy link
Contributor Author

nirarg commented Sep 5, 2022

@qinqon @mfranczy @davidvossel
The PR is ready for review

Copy link
Contributor

@qinqon qinqon left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2022
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2022
@kubevirt-bot kubevirt-bot merged commit 4fa34ca into kubevirt:main Sep 5, 2022
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. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants