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

feat: add deploy VM console proxy feature gate #609

Merged
merged 1 commit into from Jul 20, 2023
Merged

feat: add deploy VM console proxy feature gate #609

merged 1 commit into from Jul 20, 2023

Conversation

codingben
Copy link
Member

@codingben codingben commented Jun 29, 2023 •
edited

What this PR does / why we need it:

Add a new Feature Gate to deploy VM console proxy resources.

Jira-Url: https://issues.redhat.com/browse/CNV-30283

Release note:

Add deployVmConsoleProxy feature gate

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 29, 2023
@codingben codingben mentioned this pull request Jun 29, 2023
Closed
0xFelix
0xFelix reviewed Jun 29, 2023
View reviewed changes
docs/configuration.md Outdated Show resolved Hide resolved
internal/operands/vm-console-proxy/reconcile.go Outdated Show resolved Hide resolved
internal/operands/vm-console-proxy/reconcile.go Show resolved Hide resolved
akrejcir
akrejcir reviewed Jun 29, 2023
View reviewed changes
api/v1beta2/ssp_types.go Outdated Show resolved Hide resolved
api/v1beta2/ssp_types.go Outdated Show resolved Hide resolved
@codingben
Copy link
Member Author

/retest

1 similar comment
@codingben
Copy link
Member Author

/retest

@codingben
Copy link
Member Author

I've updated the PR code changes that probably will fix the failing e2e-upgrade-functests. Until then, we can wait until Andrej's PR will be merged, or merge this PR and later change it.

@0xFelix
Copy link
Member

0xFelix commented Jul 6, 2023

Let's wait until kubevirt/vm-console-proxy#10 is merged

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2023
@akrejcir
Copy link
Collaborator

Let's wait until kubevirt/vm-console-proxy#10 is merged

That PR will take some time, because of a larger change. In the meantime, you can continue working on this.
I will post a review soon.

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 17, 2023
akrejcir
akrejcir reviewed Jul 17, 2023
View reviewed changes
docs/configuration.md Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Show resolved Hide resolved
internal/operands/vm-console-proxy/reconcile.go Show resolved Hide resolved
internal/operands/vm-console-proxy/reconcile.go Outdated Show resolved Hide resolved
internal/operands/vm-console-proxy/reconcile.go Show resolved Hide resolved
@codingben codingben marked this pull request as draft July 19, 2023 12:28
@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 Jul 19, 2023
@codingben codingben marked this pull request as ready for review July 19, 2023 12:37
@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 Jul 19, 2023
@codingben
Copy link
Member Author

@akrejcir Done. Unit tests and functional tests passed locally.

@codingben
Copy link
Member Author

There is nil error in e2e-upgrade-functests job, I'll debug it tomorrow. Until then, the PR is still ready to be reviewed.

akrejcir
akrejcir reviewed Jul 19, 2023
View reviewed changes
tests/vm_console_proxy_test.go Outdated Show resolved Hide resolved
tests/vm_console_proxy_test.go Outdated Show resolved Hide resolved
@codingben
feat: add deploy VM console proxy feature gate
8551c1d 
Add a new Feature Gate to deploy VM console
proxy resources.

Jira-Url: https://issues.redhat.com/browse/CNV-30283
Signed-off-by: Ben Oukhanov <boukhanov@redhat.com>
@sonarcloud
Copy link

sonarcloud bot commented Jul 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codingben codingben requested a review from akrejcir July 20, 2023 12:08
@codingben
Copy link
Member Author

@akrejcir Thanks man! review comments resolved.

0xFelix
0xFelix reviewed Jul 20, 2023
View reviewed changes
Copy link
Member

@0xFelix 0xFelix left a comment

Choose a reason for hiding this comment

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

Looks fine to me, let's see what @akrejcir says.

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0xFelix

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 Jul 20, 2023
@akrejcir
Copy link
Collaborator

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2023
@kubevirt-bot kubevirt-bot merged commit d4cfe2c into kubevirt:main Jul 20, 2023
11 checks passed
@akrejcir
Copy link
Collaborator

/cherry-pick release-v0.18

@kubevirt-bot
Copy link
Contributor

@akrejcir: #609 failed to apply on top of branch "release-v0.18":

Applying: feat: add deploy VM console proxy feature gate
Using index info to reconstruct a base tree...
M	config/samples/ssp_v1beta2_ssp.yaml
M	data/olm-catalog/ssp-operator.clusterserviceversion.yaml
A	docs/configuration.md
M	internal/operands/vm-console-proxy/reconcile.go
Falling back to patching base and 3-way merge...
Auto-merging internal/operands/vm-console-proxy/reconcile.go
CONFLICT (modify/delete): docs/configuration.md deleted in HEAD and modified in feat: add deploy VM console proxy feature gate. Version feat: add deploy VM console proxy feature gate of docs/configuration.md left in tree.
Auto-merging data/olm-catalog/ssp-operator.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in data/olm-catalog/ssp-operator.clusterserviceversion.yaml
Auto-merging config/samples/ssp_v1beta2_ssp.yaml
CONFLICT (content): Merge conflict in config/samples/ssp_v1beta2_ssp.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 feat: add deploy VM console proxy feature gate
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:

/cherry-pick release-v0.18

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.

@akrejcir
Copy link
Collaborator

@codingben Can you manually backport this to release-v0.18?

@codingben codingben mentioned this pull request Jul 23, 2023
Merged
@codingben
Copy link
Member Author

@codingben Can you manually backport this to release-v0.18?

Done #633. Sorry I missed your comment earlier.

@codingben codingben mentioned this pull request Jul 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xFelix 0xFelix 0xFelix left review comments

@ksimon1 ksimon1 Awaiting requested review from ksimon1

@akrejcir akrejcir Awaiting requested review from akrejcir

Assignees

@akrejcir akrejcir

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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@codingben @0xFelix @akrejcir @kubevirt-bot