Skip to content

Conversation

@pierreprinetti
Copy link
Member

@pierreprinetti pierreprinetti commented Mar 1, 2022

Before this patch, the e2e tests pulled the latest tag of
quay.io/keycloak/keycloak. The upgrade of the latest image to
Keycloack v17.0.0 made the e2e test suite fail.

More specifically, in the same conditions the 17.0.0 image does not
start automatically
any more when the container is created.

This patch pins the Keycloak version to the "legacy" variant of v17.0.0,
which exposes the same behaviour as what the tests expect.

@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Mar 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2022

@pierreprinetti: This pull request references Bugzilla bug 2059515, which is invalid:

  • expected the bug to target the "4.11.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

WIP: Bug 2059515: e2e: Pin the Keycloak version used in tests

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 openshift-ci bot requested review from stlaz and sttts March 1, 2022 14:53
@pierreprinetti
Copy link
Member Author

pierreprinetti commented Mar 1, 2022

In a separate patch, I will propose to instrument 4.11 to use the non-legacy variant of Keycloak v17, and keep some form of pinning.

/retitle Bug 2059515: e2e: Pin the Keycloak version used in tests
/bugzilla refresh
/cc s-urbaniak

@openshift-ci openshift-ci bot requested a review from s-urbaniak March 1, 2022 16:50
@openshift-ci openshift-ci bot changed the title WIP: Bug 2059515: e2e: Pin the Keycloak version used in tests Bug 2059515: e2e: Pin the Keycloak version used in tests Mar 1, 2022
@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Mar 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2022

@pierreprinetti: This pull request references Bugzilla bug 2059515, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @xingxingxia

In response to this:

More specifically, the 17.0.0 image does not start automatically when the container is created.

Here I propose to pin the Keycloak version anyway and backport the change down to v4.7 to unblock any other patches. In a separate patch, we can instrument 4.11 to actually use Keycloak v17 (by passing the start or start-dev command).

One alternative can be to slap start-dev here and in the backports.

/retitle Bug 2059515: e2e: Pin the Keycloak version used in tests
/bugzilla refresh
/cc s-urbaniak

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 openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 1, 2022
@openshift-ci openshift-ci bot requested a review from xingxingxia March 1, 2022 16:50
@pierreprinetti pierreprinetti force-pushed the bug2059515 branch 3 times, most recently from 26ab13b to 6439073 Compare March 1, 2022 20:23
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2022

@pierreprinetti: This pull request references Bugzilla bug 2059515, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @xingxingxia

In response to this:

Bug 2059515: e2e: Pin the Keycloak version used in tests

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.

@pierreprinetti
Copy link
Member Author

/assign stlaz

@xingxingxia
Copy link
Contributor

It was raised in keycloak/keycloak-quickstarts#300 but no update yet, @pierreprinetti FYI.

@pierreprinetti
Copy link
Member Author

/retest

@pierreprinetti
Copy link
Member Author

/retest
/cherry-pick release-4.10

@openshift-cherrypick-robot

@pierreprinetti: once the present PR merges, I will cherry-pick it on top of release-4.10 in a new PR and assign it to you.

In response to this:

/retest
/cherry-pick release-4.10

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.

@s-urbaniak
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 2, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pierreprinetti, s-urbaniak
To complete the pull request process, please ask for approval from stlaz after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

@pierreprinetti
Copy link
Member Author

/retest-required

1 similar comment
@pierreprinetti
Copy link
Member Author

/retest-required

@stlaz
Copy link
Contributor

stlaz commented Mar 3, 2022

/hold
share:

  1. issue in a Keycloak issue tracker that shows that the developers are aware of the problem. Latest openshift-examples/keycloak.yaml of 17.0.0 does not work keycloak/keycloak-quickstarts#300 is not enough because nobody has looked into it for 14 days. Contact the developers personally (we have the choice in RH, fortunately), to check they are aware of the issue and see when they are going to fix it if Latest openshift-examples/keycloak.yaml of 17.0.0 does not work keycloak/keycloak-quickstarts#300 is the only tracker for it

  2. open a release-blocking BZ to move back to the latest image tag. We want to be testing with the latest Keycloak releases to make sure our functionality does not break with their new release

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2022
@pierreprinetti
Copy link
Member Author

@stlaz
Thank you for the swift review.

As mentioned in the BZ, I have already contacted the Keycloak devs. They mentioned that v17 introduced an API change and there's no intention to come back to the old behaviour (see the internal conversation in the chat).

Since you don't want to pin the test dependency, I am going to change this PR to apply the alternative solution, which is: change the tests to use the new (v17, Quarkus-based) API.

My understanding is that you expect the API change to also be backported; that's what I am now going for.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 3, 2022

New changes are detected. LGTM label has been removed.

Before this patch, the e2e tests pulled the `latest` tag of
`quay.io/keycloak/keycloak`. The upgrade of the `latest` image to
Keycloack v17.0.0 made the e2e test suite fail.

More specifically, in the same conditions the 17.0.0 image [does not
start automatically][1] any more when the container is created.

This patch pins the Keycloak version to the "legacy" variant of v17.0.0,
which exposes the same behaviour as what the tests expect.

[1]: keycloak/keycloak-containers@41baf7c
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 3, 2022

@pierreprinetti: 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/e2e-operator 3f5f261 link true /test e2e-operator
ci/prow/e2e-agnostic 3f5f261 link true /test e2e-agnostic
ci/prow/e2e-agnostic-ipv6 3f5f261 link false /test e2e-agnostic-ipv6
ci/prow/e2e-agnostic-upgrade 3f5f261 link true /test e2e-agnostic-upgrade
ci/prow/e2e-aws-single-node 3f5f261 link false /test e2e-aws-single-node
ci/prow/e2e-console-login 3f5f261 link true /test e2e-console-login

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.

@pierreprinetti
Copy link
Member Author

/close
Superseded by #554

@openshift-ci openshift-ci bot closed this Mar 3, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 3, 2022

@pierreprinetti: Closed this PR.

In response to this:

/close
Superseded by #554

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
Copy link
Contributor

openshift-ci bot commented Mar 3, 2022

@pierreprinetti: This pull request references Bugzilla bug 2059515. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

Bug 2059515: e2e: Pin the Keycloak version used in tests

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants