Runtime 15 e2e #1043

ranakan19 · 2024-08-28T01:02:31Z

Changes this PR includes:

Updates k8s dependencies to 0.27 and controller-runtime to v0.15.
With the update to controller-runtime v0.15 (changes in release detailed here), this PR has following changes to combat the breaking changes:
- With the changes to rest mapper that provided client for discovery information to do REST mappings, breaks the use case of checking available APIs via k8s.io/apimachinery/pkg/api/errors.IsNotFound and k8s.io/apimachinery/pkg/api/meta.IsNoMatchError. This means can not directly check for those errors, but should expect server error. This only affected proxy tests and the tests now check for error contains. Can find more details in this issue
Poll is deprecated, so using the suggested replacement PollUntillContextTimeout instead
- Tried to propogate the context from PollUntillContextTimeout to the api calls within the condition function but it fails withwould exceed context deadlineas the last error instead of the expected context deadline exceeded error. Please refer this comment for more details. And other PR which use the same solution of intentionally pass a separate context to api calls in the consition function.
- PollUntillContextTimeout will no longer return ErrWaitTimeout - So replaces errTimeout checks with context deadline checks. [refer this PR for more information if needed]

… into runtime_15_e2e

openshift-ci · 2024-08-28T01:02:35Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-08-28T01:02:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ranakan19

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

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

ranakan19 · 2024-08-28T02:10:40Z

testsupport/wait/awaitility.go

@@ -440,9 +440,10 @@ func (a *Awaitility) CreateNamespace(t *testing.T, name string) {
 func (a *Awaitility) WaitForDeploymentToGetReady(t *testing.T, name string, replicas int, criteria ...DeploymentCriteria) *appsv1.Deployment {
 	t.Logf("waiting until deployment '%s' in namespace '%s' is ready", name, a.Namespace)
 	deployment := &appsv1.Deployment{}
-	err := wait.Poll(a.RetryInterval, 6*a.Timeout, func() (done bool, err error) {
+	err := wait.PollUntilContextTimeout(context.TODO(), a.RetryInterval, 6*a.Timeout, true, func(ctx context.Context) (done bool, err error) {


Poll is deprecated

…Get, revert retryInterval

…lUntillContextTimeoout

…nto runtime_15_e2e

…pplication

…client

ranakan19 · 2024-10-02T14:08:53Z

test/e2e/user_management_test.go

@@ -590,7 +590,7 @@ func (s *userManagementTestSuite) TestUserBanning() {
 		t.Logf("user signup '%s' created", userSignup.Name)

 		// Check the UserSignup is created
-		userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name)
+		userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueBanned))


wait for Label to avoid flakiness
Since in the following lines, the test checks that usersignup now has label.
PR failure - https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/codeready-toolchain_toolchain-e2e/1043/pull-ci-codeready-toolchain-toolchain-e2e-master-e2e/1841349548585259008 - but was unable to produce locally, so added a wait for label.

It makes sense, however we don't need lines 597-599 anymore.

ranakan19 · 2024-10-04T02:11:03Z

test/e2e/parallel/proxy_test.go

@@ -365,11 +364,12 @@ func TestProxyFlow(t *testing.T) {
 				require.NoError(t, err)
 				err = proxyCl.Create(context.TODO(), appToCreate)

-				// then


This comment block is removed as it is no longer valid. Maintaining history here might be confusing

… into runtime_15_e2e

sonarcloud · 2024-10-04T14:26:03Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.4% Duplication on New Code

See analysis details on SonarCloud

alexeykazakov

Looks good. I left a few minor comments though.

alexeykazakov · 2024-10-04T22:55:07Z

test/e2e/parallel/proxy_test.go

@@ -513,7 +519,8 @@ func TestProxyFlow(t *testing.T) {
 			// note: the actual error message is "invalid workspace request: access to workspace '%s' is forbidden" but clients make api discovery calls
 			// that fail because in this case the api discovery calls go through the proxyWorkspaceURL which is invalid. If using oc or kubectl and you
 			// enable verbose logging you would see Response Body: invalid workspace request: access to workspace 'proxymember2' is forbidden
-			require.EqualError(t, err, `no matches for kind "Application" in version "appstudio.redhat.com/v1alpha1"`)
+
+			require.ErrorContains(t, err, "invalid workspace request: access to workspace 'proxymember2' is forbidden")


We can delete the comment above as irrelevant now.

alexeykazakov · 2024-10-04T22:56:40Z

test/e2e/parallel/proxy_test.go

@@ -1064,22 +1071,22 @@ func TestSpaceLister(t *testing.T) {
 			err := proxyClDefault.Get(context.TODO(), workspaceNamespacedName, &toolchainv1alpha1.Workspace{})

 			require.Error(t, err)
-			require.True(t, k8serr.IsForbidden(err), "expected Forbidden error, got %v", err)
+			require.ErrorContains(t, err, "user access is forbidden")


Isn't the error type is ForbiddenError anymore?

alexeykazakov · 2024-10-04T23:02:51Z

test/e2e/proxy_publicviewer_test.go

-		testCases := map[string]bool{
-			"user is not banned": false,
-			"user is banned":     true,
+		testCases := []TestCase{


You can use the following format instead:

tests := map[string]struct { Banned bool ExpectedError string }{ "user is not banned": { Banned: false, ExpectedError: "invalid workspace request", }, }

alexeykazakov · 2024-10-04T23:04:10Z

test/e2e/proxy_publicviewer_test.go

@@ -255,7 +267,7 @@ func TestProxyPublicViewer(t *testing.T) {
 								cms := corev1.ConfigMapList{}

 								err = communityUserProxyClient.List(context.TODO(), &cms, client.InNamespace(sp.Status.ProvisionedNamespaces[0].Name))
-								require.True(t, meta.IsNoMatchError(err), "expected List ConfigMap as community user to return a NoMatch error, actual: %v", err)
+								require.ErrorContains(t, err, testCase.expectedError)


Should we use require.ErrorEquals() instead?

alexeykazakov · 2024-10-04T23:07:50Z

test/e2e/user_management_test.go

@@ -590,7 +590,7 @@ func (s *userManagementTestSuite) TestUserBanning() {
 		t.Logf("user signup '%s' created", userSignup.Name)

 		// Check the UserSignup is created
-		userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name)
+		userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name, wait.UntilUserSignupHasStateLabel(toolchainv1alpha1.UserSignupStateLabelValueBanned))


It makes sense, however we don't need lines 597-599 anymore.

ranakan19 added 4 commits August 14, 2024 23:25

upgrade controller-runtime version

61deb41

use polluntillContext when poll is deprecated

46487e2

Merge branch 'master' of github.com:codeready-toolchain/toolchain-e2e…

f070eb6

… into runtime_15_e2e

update mod

9541f3a

openshift-ci bot added the do-not-merge/work-in-progress label Aug 28, 2024

openshift-ci bot added the approved label Aug 28, 2024

ranakan19 commented Aug 28, 2024

View reviewed changes

ranakan19 marked this pull request as ready for review August 28, 2024 02:42

ranakan19 requested review from MatousJobanek, xcoulon, alexeykazakov, rajivnathan, sbryzak and mfrancisc as code owners August 28, 2024 02:42

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 28, 2024

ranakan19 and others added 8 commits August 28, 2024 20:22

remove deprecated poll and waitforError

4f41fbc

replace timeout error check with context deadline check

d721a79

use context error instead

6a7f9f1

Merge branch 'master' into runtime_15_e2e

765b3d1

try matching interval time to client-go

d851aa1

try using new context within PollUntilCOntextTimeout to be used with …

d578dd1

…Get, revert retryInterval

Merge branch 'master' into runtime_15_e2e

1cc7262

correct version of common in replace after merge

0fbde88

openshift-merge-robot added the needs-rebase label Sep 9, 2024

ranakan19 and others added 2 commits September 12, 2024 19:18

remove propagated context when api client call is being made with pol…

9a98bd5

…lUntillContextTimeoout

Merge branch 'master' into runtime_15_e2e

8f78c2d

openshift-merge-robot removed the needs-rebase label Sep 22, 2024

correct replace version after pull

0c918fd

ranakan19 and others added 2 commits September 23, 2024 11:42

update api and common

1fdbf9d

Merge branch 'master' into runtime_15_e2e

af61bf3

openshift-merge-robot added the needs-rebase label Sep 27, 2024

ranakan19 and others added 3 commits September 30, 2024 13:01

fix proxy tests

35bc45e

Merge branch 'runtime_15_e2e' of github.com:ranakan19/toolchain-e2e i…

0bb0e75

…nto runtime_15_e2e

Merge branch 'master' into runtime_15_e2e

05fe4ab

openshift-merge-robot removed the needs-rebase label Sep 30, 2024

ranakan19 added 7 commits September 30, 2024 13:57

tidy

5e2b717

unable to replicate error locally, try getting workspace instead of a…

8d54f27

…pplication

fix proxy test

9c45a85

fix lint

80e60f6

replace k8s isForbidden error comparision to error contains for proxy…

0e426c0

…client

fix proxyPublicviewer_test

c28e778

add wait for label to avoid flakiness

d436378

ranakan19 commented Oct 2, 2024

View reviewed changes

remove comment which is no longer valid, and update comment msg

14f4c0f

ranakan19 commented Oct 4, 2024

View reviewed changes

Merge branch 'master' of github.com:codeready-toolchain/toolchain-e2e…

a12e641

… into runtime_15_e2e

alexeykazakov reviewed Oct 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Runtime 15 e2e #1043

Runtime 15 e2e #1043

ranakan19 commented Aug 28, 2024 •

edited

Loading

openshift-ci bot commented Aug 28, 2024

openshift-ci bot commented Aug 28, 2024

ranakan19 Aug 28, 2024

ranakan19 Oct 2, 2024

alexeykazakov Oct 4, 2024

ranakan19 Oct 4, 2024

sonarcloud bot commented Oct 4, 2024

alexeykazakov left a comment

alexeykazakov Oct 4, 2024

alexeykazakov Oct 4, 2024

alexeykazakov Oct 4, 2024

alexeykazakov Oct 4, 2024

alexeykazakov Oct 4, 2024

Runtime 15 e2e #1043

Are you sure you want to change the base?

Runtime 15 e2e #1043

Conversation

ranakan19 commented Aug 28, 2024 • edited Loading

openshift-ci bot commented Aug 28, 2024

openshift-ci bot commented Aug 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 4, 2024

Quality Gate passed

alexeykazakov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranakan19 commented Aug 28, 2024 •

edited

Loading