test: add vm-console-proxy functional tests #498

codingben · 2023-02-15T16:30:02Z

What this PR does / why we need it:

This PR will add vm-console-proxy functional tests.

Release note:

Add vm-console-proxy functional tests

tests/vm_console_proxy_test.go

akrejcir

Can you make 2 commits in this PR? One will fix some bugs from the previous PR, like missing UpdateFuncs, and the other will add functional tests?

internal/operands/vm-console-proxy/reconcile.go

tests/vm_console_proxy_test.go

internal/operands/vm-console-proxy/reconcile.go

internal/operands/vm-console-proxy/reconcile_test.go

tests/tests_suite_test.go

tests/vm_console_proxy_test.go

codingben · 2023-02-17T11:37:52Z

@akrejcir Thanks for the review, I'll take care of it on Sunday. Also I see this PR is affected by some issue:

error: unable to read URL "https://github.com/kubevirt/ssp-operator/releases/download/null/ssp-operator.yaml", server reported 404 Not Found

lyarwood · 2023-02-17T11:40:23Z

@akrejcir Thanks for the review, I'll take care of it on Sunday. Also I see this PR is affected by some issue:
error: unable to read URL "https://github.com/kubevirt/ssp-operator/releases/download/null/ssp-operator.yaml", server reported 404 Not Found

Should be resolved, see #499

akrejcir · 2023-02-17T11:41:09Z

/retest

akrejcir · 2023-02-20T09:39:55Z

The CI is timeouting. I've created a PR to increase the timeout: openshift/release#36544

internal/operands/vm-console-proxy/reconcile.go

tests/vm_console_proxy_test.go

akrejcir · 2023-02-21T09:11:47Z

/retest

codingben · 2023-02-21T11:43:58Z

@akrejcir Looks like it failed again because of timeout? I see it here [1]:

�[38;5;9m�[1mRan 61 of 266 Specs in 3600.077 seconds�[0m
�[38;5;9m�[1mFAIL! - Interrupted by Timeout�[0m -- �[38;5;10m�[1m60 Passed�[0m | �[38;5;9m�[1m1 Failed�[0m | �[38;5;11m�[1m1 Pending�[0m | �[38;5;14m�[1m204 Skipped�[0m

[1] https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/kubevirt_ssp-operator/498/pull-ci-kubevirt-ssp-operator-master-e2e-functests/1627959301203038208

akrejcir · 2023-02-21T12:02:03Z

Let's try again, maybe it takes a while until the configuration is applied.

/retest

codingben · 2023-02-21T12:15:54Z

@akrejcir Okay. I see all *-apply jobs passed openshift/release@232d237.

akrejcir · 2023-02-21T15:01:30Z

The timeout was probably not set correctly. I've created another PR to fix it: openshift/release#36609

akrejcir · 2023-02-21T15:35:29Z

/retest

lyarwood · 2023-02-21T18:15:11Z

I think the timeout here is coming from ginkgo itself right?

https://onsi.github.io/ginkgo/#recommended-continuous-integration-configuration

--timeout allows you to specify a timeout for the ginkgo run. The default duration is one hour, which may or may not be enough!

The fact we are still using go test to launch these tests and pass a timeout of 0 (unlimited?) is likely causing this to default to the 1 hour we are seeing.

go test -v -coverprofile cover.out -timeout 0 ./tests/...

lyarwood · 2023-02-21T18:47:11Z

#504 - I wonder if this could help?

lyarwood · 2023-02-21T19:27:02Z

Okay I'm pretty sure this is a change in behaviour in v2 thanks to the following docs:

https://onsi.github.io/ginkgo/MIGRATING_TO_V2#timeout-behavior

Timeout Behavior

In Ginkgo V1.x, Ginkgo's timeout was managed by go test. This meant that timeouts exited the test suite abruptly with 
no opportunity for custom reporters or clean up code (e.g. AfterEach, AfterSuite) to run. This is fixed in V2. Ginkgo now 
manages its own timeout and when a timeout triggers the test winds down gracefully. In fact, a timeout is now 
functionally equivalent to a user-initiated interrupt.

In addition, in V1.x when running multiple test suites Ginkgo would give each suite the full timeout allotment (so ginkgo -
r -timeout=1h would give each test suite one hour to complete). In V2 the timeout now applies to the entire test suite run 
so that ginkgo -r -timeout=1h is now guaranteed to exit after (about) one hour.

Finally, the default timeout has been reduced from 24h down to 1h. Users with long-running tests may need to adjust the timeout in their CI scripts.

akrejcir · 2023-02-23T15:45:07Z

/lgtm
/approve

kubevirt-bot · 2023-02-23T15:45:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akrejcir

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~~ [akrejcir]

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

codingben · 2023-02-23T18:49:01Z

Looks like it failed because of some internal error [1]:

Status: "Failure",
              Message: "Internal error occurred: failed calling webhook \"validation.ssp.kubevirt.io\": failed to call webhook: Post \"https://ssp-webhook-service.kubevirt.svc:443/validate-ssp-kubevirt-io-v1beta1-ssp?timeout=10s\": no endpoints available for service \"ssp-webhook-service\"",
              Reason: "InternalError",

[1] https://storage.googleapis.com/origin-ci-test/pr-logs/pull/kubevirt_ssp-operator/498/pull-ci-kubevirt-ssp-operator-master-e2e-upgrade-functests/1628771659840229376/build-log.txt

codingben · 2023-02-23T18:49:08Z

/retest

codingben · 2023-02-24T10:32:46Z

Another timeout issue:

�[0mDataSources �[38;5;243mresource deletion �[0mrecreate after delete�[0m
  �[1m[test_id:4771]edit cluster role�[0m
  �[38;5;243m/go/src/github.com/kubevirt/ssp-operator/tests/dataSources_test.go:165�[0m
{"component":"entrypoint","file":"k8s.io/test-infra/prow/entrypoint/run.go:164","func":"k8s.io/test-infra/prow/entrypoint.Options.ExecuteProcess","level":"error","msg":"Process did not finish before 2h0m0s timeout","severity":"error","time":"2023-02-23T21:25:58Z"}

And:

�[0mMetrics �[38;5;243mSSP alerts rules�[0m
  �[1m[test_id:8955]should have all the required labels�[0m
  �[38;5;243m/go/src/github.com/kubevirt/ssp-operator/tests/metrics_test.go:277�[0m
{"component":"entrypoint","file":"k8s.io/test-infra/prow/entrypoint/run.go:164","func":"k8s.io/test-infra/prow/entrypoint.Options.ExecuteProcess","level":"error","msg":"Process did not finish before 2h0m0s timeout","severity":"error","time":"2023-02-23T21:37:46Z"}

codingben · 2023-02-24T10:34:09Z

/test e2e-functests
/test e2e-single-node-functests

kubevirt-bot · 2023-02-24T10:34:12Z

@codingben: No presubmit jobs available for kubevirt/ssp-operator@master

In response to this:

/test e2e-functests
/test e2e-single-node-functests

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 · 2023-02-27T15:22:23Z

internal/operands/vm-console-proxy/reconcile.go

 		return common.CreateOrUpdate(request).
 			ClusterResource(&serviceAccount).
 			WithAppLabels(operandName, operandComponent).
+			UpdateFunc(func(newRes, foundRes client.Object) {


I was mistaken about this change. The cluster adds a secret to the service account by default. This default field causes a reconciliation loop, and is removed again here in this function.

The loop means that SSP is always in a Deploying state. Curiously, the functional tests should fail after 10 minutes if Deployed state is not reached, but because of a race, sometimes the Deployed state is seen and that is enough for the test to progress.

Please remove this UpdateFunc().

Makefile

akrejcir · 2023-02-27T17:30:46Z

/retest

codingben · 2023-02-27T19:18:55Z

It's failing now because of some weird error:

{  could not initialize namespace: failed to wait for authentication cache to warm up after 15s: projects.project.openshift.io "ci-op-ngsfcfg0" is forbidden: User "system:serviceaccount:ci:ci-operator" cannot get resource "projects" in API group "project.openshift.io" in the namespace "ci-op-ngsfcfg0"}

codingben · 2023-02-27T19:19:04Z

/retest

lyarwood · 2023-02-28T10:04:35Z

/retest-required

akrejcir · 2023-02-28T10:45:24Z

Retest will not help. I'm looking into it.

internal/operands/vm-console-proxy/reconcile.go

akrejcir · 2023-02-28T12:10:13Z

And just to be safe of unexpected changes, can you make a DeepCopy() of all the objects used in Reconcile() function?
The shallow copies that are used now contain pointers, and can save some state from one reconciliation iteration to another.

Add vm-console-proxy functional tests. Signed-off-by: Ben Oukhanov <[email protected]>

sonarqubecloud · 2023-02-28T12:21:07Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

codingben · 2023-02-28T12:22:12Z

@akrejcir Thanks!

akrejcir · 2023-02-28T12:22:18Z

/lgtm

akrejcir · 2023-02-28T14:26:30Z

/retest

akrejcir · 2023-02-28T14:32:10Z

/retest

codingben · 2023-02-28T17:02:26Z

/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 Feb 15, 2023

kubevirt-bot requested review from ksimon1 and lyarwood February 15, 2023 16:30

kubevirt-bot added the size/L label Feb 15, 2023

akrejcir added this to the v0.17.0 milestone Feb 16, 2023

akrejcir reviewed Feb 16, 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 requested review from akrejcir and removed request for lyarwood and ksimon1 February 16, 2023 12:24

akrejcir reviewed Feb 16, 2023

View reviewed changes

internal/operands/vm-console-proxy/reconcile.go Show resolved Hide resolved

internal/operands/vm-console-proxy/reconcile.go Show resolved Hide resolved

codingben requested a review from akrejcir February 16, 2023 15:22

akrejcir reviewed Feb 17, 2023

View reviewed changes

codingben requested a review from akrejcir February 19, 2023 13:21

akrejcir reviewed Feb 20, 2023

View reviewed changes

internal/operands/vm-console-proxy/reconcile.go Outdated Show resolved Hide resolved

tests/vm_console_proxy_test.go Outdated Show resolved Hide resolved

tests/vm_console_proxy_test.go Outdated Show resolved Hide resolved

codingben requested a review from akrejcir February 20, 2023 11:56

codingben mentioned this pull request Feb 21, 2023

Add vm-console-proxy #502

Closed

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

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

akrejcir reviewed Feb 27, 2023

View reviewed changes

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 27, 2023

codingben requested a review from akrejcir February 27, 2023 16:20

akrejcir reviewed Feb 28, 2023

View reviewed changes

internal/operands/vm-console-proxy/reconcile.go Outdated Show resolved Hide resolved

test: add vm-console-proxy functional tests

749cf04

Add vm-console-proxy functional tests. Signed-off-by: Ben Oukhanov <[email protected]>

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

akrejcir mentioned this pull request Feb 28, 2023

feat: Add Route to expose vm-console-proxy #507

Merged

kubevirt-bot merged commit ccf3d9f into kubevirt:master Feb 28, 2023

codingben deleted the CNV-25394 branch June 28, 2023 14:22

test: add vm-console-proxy functional tests #498

test: add vm-console-proxy functional tests #498

Conversation

codingben commented Feb 15, 2023

akrejcir left a comment

Choose a reason for hiding this comment

codingben commented Feb 17, 2023

lyarwood commented Feb 17, 2023

akrejcir commented Feb 17, 2023

akrejcir commented Feb 20, 2023

akrejcir commented Feb 21, 2023

codingben commented Feb 21, 2023

akrejcir commented Feb 21, 2023

codingben commented Feb 21, 2023

akrejcir commented Feb 21, 2023

akrejcir commented Feb 21, 2023

lyarwood commented Feb 21, 2023

lyarwood commented Feb 21, 2023

lyarwood commented Feb 21, 2023 • edited Loading

akrejcir commented Feb 23, 2023

kubevirt-bot commented Feb 23, 2023

codingben commented Feb 23, 2023

codingben commented Feb 23, 2023

codingben commented Feb 24, 2023 • edited Loading

codingben commented Feb 24, 2023

kubevirt-bot commented Feb 24, 2023

akrejcir Feb 27, 2023

Choose a reason for hiding this comment

codingben Feb 27, 2023

Choose a reason for hiding this comment

akrejcir commented Feb 27, 2023

codingben commented Feb 27, 2023

codingben commented Feb 27, 2023

lyarwood commented Feb 28, 2023

akrejcir commented Feb 28, 2023

akrejcir commented Feb 28, 2023 • edited Loading

sonarqubecloud bot commented Feb 28, 2023 • edited Loading

codingben commented Feb 28, 2023

akrejcir commented Feb 28, 2023

akrejcir commented Feb 28, 2023

akrejcir commented Feb 28, 2023

codingben commented Feb 28, 2023

lyarwood commented Feb 21, 2023 •

edited

Loading

codingben commented Feb 24, 2023 •

edited

Loading

akrejcir commented Feb 28, 2023 •

edited

Loading

sonarqubecloud bot commented Feb 28, 2023 •

edited

Loading