Add apiserver-proxy for KubeVirt UI Plugin #2401

orenc1 · 2023-06-25T19:19:50Z

An apiserver proxy pod is being introduced to improve API queries performance for virtual machines resources done by the UI client.

VirtualMachines table is getting data from the apiserver proxy:

What this PR does / why we need it:

Reviewer Checklist

Reviewers are supposed to review the PR for every aspect below one by one. To check an item means the PR is either "OK" or "Not Applicable" in terms of that item. All items are supposed to be checked before merging a PR.

Jira Ticket:

https://issues.redhat.com/browse/CNV-30060

Release note:

Add apiserver-proxy for KubeVirt UI Plugin

hco-bot · 2023-06-25T20:38:51Z

hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure

kubevirt-bot · 2023-06-25T20:38:53Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-azure

In response to this:

hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure

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.

hco-bot · 2023-06-25T21:16:44Z

hco-e2e-operator-sdk-gcp, hco-e2e-operator-sdk-aws lanes succeeded.
/override ci/prow/hco-e2e-operator-sdk-azure

kubevirt-bot · 2023-06-25T21:16:47Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-azure

In response to this:

hco-e2e-operator-sdk-gcp, hco-e2e-operator-sdk-aws lanes succeeded.
/override ci/prow/hco-e2e-operator-sdk-azure

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.

hco-bot · 2023-06-25T22:52:24Z

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

kubevirt-bot · 2023-06-25T22:52:27Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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.

coveralls · 2023-06-26T07:04:51Z

Pull Request Test Coverage Report for Build 5402102077

99 of 101 (98.02%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 86.092%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/util/cluster.go	0	2	0.0%

Totals
Change from base Build 5399009068:	0.2%
Covered Lines:	5008
Relevant Lines:	5817

💛 - Coveralls

hco-bot · 2023-06-26T10:22:29Z

okd-hco-e2e-operator-sdk-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-aws

kubevirt-bot · 2023-06-26T10:22:33Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/okd-hco-e2e-operator-sdk-aws

In response to this:

okd-hco-e2e-operator-sdk-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-aws

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.

hco-bot · 2023-06-26T13:58:15Z

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

kubevirt-bot · 2023-06-26T13:58:18Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/okd-hco-e2e-operator-sdk-gcp

In response to this:

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

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.

hco-bot · 2023-06-26T14:23:01Z

hco-e2e-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-gcp
hco-e2e-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-aws
okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

kubevirt-bot · 2023-06-26T14:23:06Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-aws, ci/prow/hco-e2e-operator-sdk-gcp, ci/prow/okd-hco-e2e-operator-sdk-gcp

In response to this:

hco-e2e-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-gcp
hco-e2e-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-aws
okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

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.

hco-bot · 2023-06-27T23:48:26Z

hco-e2e-upgrade-index-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-azure

kubevirt-bot · 2023-06-27T23:48:29Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-sno-azure

In response to this:

hco-e2e-upgrade-index-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-azure

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.

hco-bot · 2023-06-28T03:08:06Z

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

kubevirt-bot · 2023-06-28T03:08:09Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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.

nunnatsa

Overall looks good.

My main comment is about the DescribeTable tests, where the code keep asking about thing it can get from the outside - this is the whole idea of the parameterized tests. To make it easier, let's get rid of the error return values from some creation functions, that will never return an error.

nunnatsa · 2023-06-28T05:08:20Z

controllers/operands/kubevirtConsolePlugin.go

+		kvUIProxyServingCertPath, hcoutil.UIProxyServerPort, hcoutil.AppComponentUIProxy)
+}
+
+func getKvUIDeployment(hc *hcov1beta1.HyperConverged, deploymentName string, image string,


This function never returns error. can we get rid of the error return value?

nunnatsa · 2023-06-28T05:09:04Z

controllers/operands/kubevirtConsolePlugin.go

@@ -57,14 +71,53 @@ func newKvUIPluginCRHandler(_ log.Logger, Client client.Client, Scheme *runtime.
 	return []Operand{newConsolePluginHandler(Client, Scheme, kvUIConsolePluginCR)}, nil
 }

-func NewKvUIPluginDeplymnt(hc *hcov1beta1.HyperConverged) (*appsv1.Deployment, error) {
+func NewKvUIPluginDeployment(hc *hcov1beta1.HyperConverged) (*appsv1.Deployment, error) {


This function never returns error. can we get rid of the error return value?

controllers/operands/kubevirtConsolePlugin.go

nunnatsa · 2023-06-28T05:13:09Z

controllers/operands/kubevirtConsolePlugin_test.go

+			var err error
+			switch appComponent {
+			case hcoutil.AppComponentUIPlugin:
+				expectedResource, err = NewKvUIPluginDeployment(hco)
+			case hcoutil.AppComponentUIProxy:
+				expectedResource, err = NewKvUIProxyDeployment(hco)
+			}
 			Expect(err).ToNot(HaveOccurred())


Please consider to pass the deployment as a parameter. This will be possible if we'll get rid of all the return values above. The result will be cleaner code, without code branches.

nunnatsa · 2023-06-28T05:17:35Z

controllers/operands/kubevirtConsolePlugin_test.go

-				handler, err := newKvUIPluginDeploymentHandler(logger, cl, commontestutils.GetScheme(), hco)
+				var handler []Operand
+				switch appComponent {
+				case hcoutil.AppComponentUIPlugin:
+					handler, err = newKvUIPluginDeploymentHandler(logger, cl, commontestutils.GetScheme(), hco)
+				case hcoutil.AppComponentUIProxy:
+					handler, err = newKvUIProxyDeploymentHandler(logger, cl, commontestutils.GetScheme(), hco)
+				}
 				Expect(err).ToNot(HaveOccurred())


nunnatsa · 2023-06-28T05:17:50Z

controllers/operands/kubevirtConsolePlugin_test.go

+			switch appComponent {
+			case hcoutil.AppComponentUIPlugin:
+				expectedResource = NewKvUIPluginSvc(hco)
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIPluginSvc))
+			case hcoutil.AppComponentUIProxy:
+				expectedResource = NewKvUIProxySvc(hco)
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIProxySvc))
+			}


nunnatsa · 2023-06-28T05:18:12Z

controllers/operands/kubevirtConsolePlugin_test.go

+			switch appComponent {
+			case hcoutil.AppComponentUIPlugin:
+				expectedResource = NewKvUIPluginSvc(hco)
+				cl = commontestutils.InitClient([]client.Object{hco, expectedResource})
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIPluginSvc))
+			case hcoutil.AppComponentUIProxy:
+				expectedResource = NewKvUIProxySvc(hco)
+				cl = commontestutils.InitClient([]client.Object{hco, expectedResource})
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIProxySvc))


nunnatsa · 2023-06-28T05:18:24Z

controllers/operands/kubevirtConsolePlugin_test.go

+			switch appComponent {
+			case hcoutil.AppComponentUIPlugin:
+				expectedResource = NewKvUIPluginSvc(hco)
+				outdatedResource = NewKvUIPluginSvc(hco)
+			case hcoutil.AppComponentUIProxy:
+				expectedResource = NewKvUIProxySvc(hco)
+				outdatedResource = NewKvUIProxySvc(hco)


nunnatsa · 2023-06-28T05:18:32Z

controllers/operands/kubevirtConsolePlugin_test.go

+			switch appComponent {
+			case hcoutil.AppComponentUIPlugin:
+				expectedResource = NewKvUIPluginSvc(hco)
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIPluginSvc))
+			case hcoutil.AppComponentUIProxy:
+				expectedResource = NewKvUIProxySvc(hco)
+				handler = (*genericOperand)(newServiceHandler(cl, commontestutils.GetScheme(), NewKvUIProxySvc))
+			}


Signed-off-by: Oren Cohen <ocohen@redhat.com>

sonarcloud · 2023-06-28T14:13:20Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

hco-bot · 2023-06-28T15:34:07Z

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp
hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-azure

kubevirt-bot · 2023-06-28T15:34:10Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-azure, ci/prow/okd-hco-e2e-operator-sdk-gcp

In response to this:

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp
hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-azure

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.

hco-bot · 2023-06-28T15:48:35Z

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp
okd-hco-e2e-upgrade-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-index-aws

kubevirt-bot · 2023-06-28T15:48:39Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/okd-hco-e2e-operator-sdk-gcp, ci/prow/okd-hco-e2e-upgrade-index-aws

In response to this:

okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp
okd-hco-e2e-upgrade-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-index-aws

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.

hco-bot · 2023-06-28T16:07:25Z

hco-e2e-upgrade-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-aws
hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws

kubevirt-bot · 2023-06-28T16:07:29Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-sno-aws, ci/prow/hco-e2e-upgrade-prev-index-sno-aws

In response to this:

hco-e2e-upgrade-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-aws
hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws

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.

hco-bot · 2023-06-28T17:06:40Z

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws

kubevirt-bot · 2023-06-28T17:06:43Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-aws

In response to this:

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws

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 · 2023-06-28T17:41:51Z

@orenc1: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/hco-e2e-upgrade-index-sno-aws	`3bea4ef`	link	false	`/test hco-e2e-upgrade-index-sno-aws`
ci/prow/okd-hco-e2e-upgrade-index-aws	`3bea4ef`	link	true	`/test okd-hco-e2e-upgrade-index-aws`
ci/prow/okd-hco-e2e-operator-sdk-gcp	`3bea4ef`	link	true	`/test okd-hco-e2e-operator-sdk-gcp`
ci/prow/hco-e2e-upgrade-prev-index-sno-aws	`3bea4ef`	link	false	`/test hco-e2e-upgrade-prev-index-sno-aws`
ci/prow/hco-e2e-upgrade-index-aws	`3bea4ef`	link	true	`/test hco-e2e-upgrade-index-aws`
ci/prow/hco-e2e-kv-smoke-azure	`3bea4ef`	link	true	`/test hco-e2e-kv-smoke-azure`

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.

hco-bot · 2023-06-28T17:52:57Z

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

kubevirt-bot · 2023-06-28T17:53:01Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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.

nunnatsa · 2023-06-29T05:27:22Z

/lgtm
/approve
/retest

kubevirt-bot · 2023-06-29T05:27:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nunnatsa

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:

~~OWNERS~~ [nunnatsa]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nunnatsa · 2023-06-29T05:49:33Z

/retest

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 25, 2023

kubevirt-bot requested review from nunnatsa and sradco June 25, 2023 19:19

kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L labels Jun 25, 2023

orenc1 changed the title ~~Add apiserver-proxy for KubeVirt UI Plugin~~ [WIP] Add apiserver-proxy for KubeVirt UI Plugin Jun 25, 2023

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 Jun 25, 2023

orenc1 force-pushed the ui_proxy_pod branch from e04830c to 1e0fff5 Compare June 25, 2023 19:22

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2023

orenc1 force-pushed the ui_proxy_pod branch from 1e0fff5 to e876390 Compare June 26, 2023 06:59

orenc1 force-pushed the ui_proxy_pod branch from e876390 to 7f850ad Compare June 26, 2023 07:18

orenc1 changed the title ~~[WIP] Add apiserver-proxy for KubeVirt UI Plugin~~ Add apiserver-proxy for KubeVirt UI Plugin Jun 26, 2023

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 Jun 26, 2023

orenc1 force-pushed the ui_proxy_pod branch from 7f850ad to 96c09c1 Compare June 26, 2023 08:36

orenc1 force-pushed the ui_proxy_pod branch from 96c09c1 to 0edaa68 Compare June 26, 2023 12:31

orenc1 force-pushed the ui_proxy_pod branch from 0edaa68 to b6535cc Compare June 26, 2023 14:48

nunnatsa reviewed Jun 28, 2023

View reviewed changes

Add apiserver-proxy for KubeVirt UI

3bea4ef

Signed-off-by: Oren Cohen <ocohen@redhat.com>

orenc1 force-pushed the ui_proxy_pod branch from 2692ce4 to 3bea4ef Compare June 28, 2023 14:08

kubevirt-bot added size/L and removed size/XL labels Jun 28, 2023

kubevirt-bot assigned nunnatsa Jun 29, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 29, 2023

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 29, 2023

kubevirt-bot merged commit 7215665 into kubevirt:main Jun 29, 2023
3 checks passed

orenc1 deleted the ui_proxy_pod branch June 29, 2023 07:20

dharmit mentioned this pull request Jul 31, 2023

[release-1.9] Fix node placement for kubevirt-console-plugin (#2450) #2452

Merged

11 tasks

Add apiserver-proxy for KubeVirt UI Plugin #2401

Add apiserver-proxy for KubeVirt UI Plugin #2401

Conversation

orenc1 commented Jun 25, 2023 • edited Loading

hco-bot commented Jun 25, 2023

kubevirt-bot commented Jun 25, 2023

hco-bot commented Jun 25, 2023

kubevirt-bot commented Jun 25, 2023

hco-bot commented Jun 25, 2023

kubevirt-bot commented Jun 25, 2023

coveralls commented Jun 26, 2023 • edited Loading

Pull Request Test Coverage Report for Build 5402102077

💛 - Coveralls

hco-bot commented Jun 26, 2023

kubevirt-bot commented Jun 26, 2023

hco-bot commented Jun 26, 2023

kubevirt-bot commented Jun 26, 2023

hco-bot commented Jun 26, 2023

kubevirt-bot commented Jun 26, 2023

hco-bot commented Jun 27, 2023

kubevirt-bot commented Jun 27, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

nunnatsa left a comment • edited Loading

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

nunnatsa Jun 28, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Jun 28, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

openshift-ci bot commented Jun 28, 2023

hco-bot commented Jun 28, 2023

kubevirt-bot commented Jun 28, 2023

nunnatsa commented Jun 29, 2023

kubevirt-bot commented Jun 29, 2023

nunnatsa commented Jun 29, 2023

orenc1 commented Jun 25, 2023 •

edited

Loading

coveralls commented Jun 26, 2023 •

edited

Loading

nunnatsa left a comment •

edited

Loading

nunnatsa Jun 28, 2023 •

edited

Loading