check operand StatefulSet is ready for realm import Job to run #24534

pgodowski · 2023-11-03T16:41:50Z

Check whether the Keycloak operand StatefulSet has at least single replica ready before submitting the RealmImport Job.

Closes #24526

cc @shawkins

shawkins

Thank you @pgodowski this seems good - polling is simpler than getting the Keycloak CR events.

It should have a test though.

Also I know there always should be a status added when a statefulset is created, but the code is checking for null in one place, but not the other - it should at least be consistent.

pgodowski · 2023-11-05T09:31:21Z

Thanks for the review.

Also I know there always should be a status added when a statefulset is created, but the code is checking for null in one place, but not the other - it should at least be consistent.

Fixed this part.

It should have a test though.

Will work on adding a test case in RealmImportTest.java

vmuzikar

@pgodowski Thanks for the PR!

I believe this affects main too where we don't check number of ready instances as well. We should create a parallel fix for it too to consider this fully fixed.

pgodowski · 2023-11-06T09:01:30Z

I believe this affects main too where we don't check number of ready instances as well. We should create a parallel fix for it too to consider this fully fixed.

Yes, will work on the fix in master too, but I'm super motivated to start with 22.0 branch, for obvious reasons :)

pgodowski · 2023-11-07T10:18:33Z

Added unit test to check for the failed realm import when Keycloak operand is not available, still in release/22.0 branch.

shawkins · 2023-11-09T14:14:47Z

@pgodowski looks good. Please rebase to a single commit and add your DCO signature - Keycloak has recently adopted a requirement.

vmuzikar

LGTM (once comment from @shawkins is resolved).

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

pgodowski · 2023-11-09T17:28:09Z

Please rebase to a single commit and add your DCO signature - Keycloak has recently adopted a requirement.

@shawkins done - thanks!

shawkins

LGTM. Once small nit, could there be a single crSelector.get() rather than 3 calls per polling.

pgodowski · 2023-11-10T09:05:34Z

Do you want me to add any changes here?

ghost · 2023-11-10T09:16:20Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.ui.account2.WelcomeScreenTest#accountSecurityTest

Keycloak CI - Account Console IT (firefox)

org.openqa.selenium.TimeoutException: 
Expected condition failed: waiting for wrapped: element to be clickable: GrapheneElement -> [[FirefoxDriver: firefox on LINUX (da195fcc-5609-4839-bf04-466ff6e0e49f)] -> xpath: //*[@id='landing-device-activity']/a] (tried for 5 second(s) with 500 milliseconds interval)
Build info: version: '3.14.0', revision: 'aacccce0', time: '2018-08-02T20:19:58.91Z'
System info: host: 'fv-az984-520', ip: '10.1.0.77', os.name: 'Linux', os.arch: 'amd64', os.version: '6.2.0-1015-azure', java.version: '17.0.9'
Driver info: org.openqa.selenium.firefox.FirefoxDriver$$EnhancerByGraphene$$e748af4e
...

Report flaky test

ghost

Unreported flaky test detected, please review

shawkins · 2023-11-10T17:06:39Z

Do you want me to add any changes here?

It's optional. It can be cleaned up later if needed.

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

shawkins · 2023-11-13T21:23:03Z

operator/src/test/java/org/keycloak/operator/testsuite/integration/RealmImportTest.java

+        kc.getSpec().setInstances(0);
+
+        // don't wait for Keycloak being available, since it has no instances
+        deployKeycloak(k8sclient, getDefaultKeycloakDeployment(), false);


This should reference the kc instance created above.

OMG, good catch @shawkins - fixed!

shawkins · 2023-11-13T21:36:33Z

@pgodowski I hope you don't mind, I took the liberty of opening a forward port: #24720

There's still another small change needed here.

…odowski/keycloak into fix/realm-import-prereq-check

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

pgodowski · 2023-11-14T08:58:51Z

Thanks for the forward port - let me know you need any inputs on that. Updated the test case, yet, for some reason I cannot now squash commits, hope it is fine?

shawkins

LGTM, don't worry about squashing, it will be done as part of the merge.

ghost

Unreported flaky test detected, please review

ghost · 2023-11-14T17:57:54Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginSuccessWithCRLSignedWithIntermediateCA3FromTruststore

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromFile

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromHttp

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithInvalidSignatureCRL

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginWithMultipleRevocationLists

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginSuccessWithEmptyRevocationListFromFile

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:313)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

vmuzikar · 2023-11-15T07:57:05Z

Enabled auto-merge but 22 is frozen until 22.0.6 is released.

Jamstah · 2023-11-23T12:08:19Z

As an FYI (because I'm expecting this one in 22.0.7 anyway), this is actually more dangerous than we thought.

Its not just some failed pods, but there is a timing issue where I think the realm job can trigger a database initialisation at the same time as the first Keycloak pod, and the realm status can say its ready even though its not in the database. The cause is conjecture because I don't have logs from the first keycloak pod run, but on this cluster, the realm job completed successfully but the realm was not present.

Log from a realm import pod:

Updating the configuration and installing your custom providers, if any. Please wait.
2023-11-23 09:27:46,955 INFO [io.quarkus.deployment.QuarkusAugmentor] (main) Quarkus augmentation completed in 24969ms
Server configuration updated and persisted. Run the following command to review the configuration:
kc.sh show-config
2023-11-23 09:27:50,348 INFO [org.keycloak.quarkus.runtime.hostname.DefaultHostnameProvider] (main) Hostname settings: Base URL: <unset>, Hostname: keycloak-cp4i.kepler-roks1-ec111ed5d7db435e1c5eeeb4400d693f-0000.eu-gb.containers.appdomain.cloud, Strict HTTPS: false, Path: <request>, Strict BackChannel: false, Admin URL: <unset>, Admin: <request>, Port: -1, Proxied: true
2023-11-23 09:27:53,946 WARN [io.quarkus.agroal.runtime.DataSources] (main) Datasource <default> enables XA but transaction recovery is not enabled. Please enable transaction recovery by setting quarkus.transaction-manager.enable-recovery=true, otherwise data may be lost if the application is terminated abruptly
2023-11-23 09:27:55,351 WARN [org.infinispan.PERSISTENCE] (keycloak-cache-init) ISPN000554: jboss-marshalling is deprecated and planned for removal
2023-11-23 09:27:55,554 WARN [org.infinispan.CONFIG] (keycloak-cache-init) ISPN000569: Unable to persist Infinispan internal caches as no global state enabled
2023-11-23 09:27:55,763 INFO [org.infinispan.CONTAINER] (keycloak-cache-init) ISPN000556: Starting user marshaller 'org.infinispan.jboss.marshalling.core.JBossUserMarshaller'
2023-11-23 09:27:58,180 INFO [org.keycloak.broker.provider.AbstractIdentityProviderMapper] (main) Registering class org.keycloak.broker.provider.mappersync.ConfigSyncEventListener
2023-11-23 09:27:58,263 INFO [org.keycloak.connections.infinispan.DefaultInfinispanConnectionProviderFactory] (main) Node name: node_311387, Site name: null
2023-11-23 09:28:07,869 INFO [org.keycloak.quarkus.runtime.storage.legacy.liquibase.QuarkusJpaUpdaterProvider] (main) Initializing database schema. Using changelog META-INF/jpa-changelog-master.xml
UPDATE SUMMARY
Run: 115
Previously run: 0
Filtered out: 0
-------------------------------
Total change sets: 115
2023-11-23 09:28:19,544 INFO [org.keycloak.services] (main) KC-SERVICES0050: Initializing master realm
2023-11-23 09:28:24,240 INFO [org.keycloak.exportimport.singlefile.SingleFileImportProvider] (main) Full importing from file /mnt/realm-import/cloudpak-realm.json
2023-11-23 09:28:28,364 INFO [org.keycloak.exportimport.util.ImportUtils] (main) Realm 'cloudpak' imported
2023-11-23 09:28:28,673 INFO [io.quarkus] (main) Keycloak 22.0.6.redhat-00002 on JVM (powered by Quarkus 3.2.6.Final-redhat-00002) started in 41.495s. Listening on:
2023-11-23 09:28:28,674 INFO [io.quarkus] (main) Profile import_export activated.
2023-11-23 09:28:28,674 INFO [io.quarkus] (main) Installed features: [agroal, cdi, hibernate-orm, jdbc-h2, jdbc-mariadb, jdbc-mssql, jdbc-mysql, jdbc-oracle, jdbc-postgresql, keycloak, micrometer, narayana-jta, reactive-routes, resteasy, resteasy-jackson, smallrye-context-propagation, smallrye-health, vertx]
2023-11-23 09:28:28,774 INFO [io.quarkus] (main) Keycloak stopped in 0.096s

pgodowski · 2023-11-23T13:54:10Z

As an FYI (because I'm expecting this one in 22.0.7 anyway), this is actually more dangerous than we thought.

Are you suggesting we shall check for other thing in the proposed fix than the operand StatefulSet, or it's just your notice saying that the impact is larger than we initially though?

Jamstah · 2023-11-23T14:08:22Z

As an FYI (because I'm expecting this one in 22.0.7 anyway), this is actually more dangerous than we thought.

Are you suggesting we shall check for other thing in the proposed fix than the operand StatefulSet, or it's just your notice saying that the impact is larger than we initially though?

Just that the impact is more than just a UX snag - it can leave an installation in a broken state.

I don't think we need more than a single pod ready check.

cypress · 2023-11-27T19:34:07Z

1 failed and 2 flaky tests on run #9989 ↗︎

1	526	48	0	2

Details:

check operand StatefulSet is ready for realm import Job to run (#24534)
Project: Keycloak Admin UI	Commit: `002f1a7ce6`
Status: Failed	Duration: 10:00 💡
Started: Nov 27, 2023 7:24 PM	Ended: Nov 27, 2023 7:34 PM

cypress/e2e/sessions_test.spec.ts • 1 failed test • chrome

View Output Video

Test		Artifacts
Sessions test > revocation > Check if notBefore saved		`Screenshots` `Video`

clients_test.spec.ts • 1 flaky test • chrome

View Output

Test		Artifacts
Clients test > Accessibility tests for clients > Check a11y violations on client registration/ authenticated access policies tab		`Screenshots`

authentication_test.spec.ts • 1 flaky test • chrome

View Output

Test		Artifacts
Authentication test > should add a condition		`Screenshots`

Review all test suite changes for PR #24534 ↗︎

pgodowski requested review from a team as code owners November 3, 2023 16:41

ghost added team/cloud-native labels Nov 3, 2023

pgodowski mentioned this pull request Nov 3, 2023

Keycloak operator 22.0.z created RealmImport Job withouth checking Keycloak operand health first #24526

Closed

1 task

pgodowski changed the title ~~check operand StatefulSet is ready~~ check operand StatefulSet is ready for realm import Job to run Nov 3, 2023

shawkins requested a review from vmuzikar November 3, 2023 22:33

shawkins reviewed Nov 3, 2023

View reviewed changes

vmuzikar reviewed Nov 6, 2023

View reviewed changes

ghost added the flaky-test label Nov 6, 2023

This was referenced Nov 6, 2023

Flaky test: org.keycloak.testsuite.forms.BrowserFlowTest#testAlternativeNonInteractiveExecutorInSubflow #22644

Closed

Flaky test: org.keycloak.testsuite.forms.RecoveryAuthnCodesAuthenticatorTest#test03AuthenticateRecoveryAuthnCodes #22691

Closed

ghost mentioned this pull request Nov 8, 2023

Flaky test: org.keycloak.testsuite.adapter.servlet.OfflineServletsAdapterTest#testServlet #21536

Closed

vmuzikar assigned shawkins Nov 9, 2023

vmuzikar approved these changes Nov 9, 2023

View reviewed changes

pgodowski force-pushed the fix/realm-import-prereq-check branch from a1dda82 to 1aecef1 Compare November 9, 2023 15:36

check operand StatefulSet is ready

6dd0531

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

pgodowski force-pushed the fix/realm-import-prereq-check branch from 1aecef1 to 6dd0531 Compare November 9, 2023 17:26

shawkins approved these changes Nov 9, 2023

View reviewed changes

ghost reviewed Nov 10, 2023

View reviewed changes

shawkins enabled auto-merge (squash) November 10, 2023 17:06

check operand StatefulSet is ready

9c7519f

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

shawkins force-pushed the fix/realm-import-prereq-check branch from 6dd0531 to 9c7519f Compare November 10, 2023 18:46

ghost mentioned this pull request Nov 10, 2023

Flaky test: org.keycloak.testsuite.forms.ResetPasswordTest#resetPasswordLink #21422

Closed

shawkins disabled auto-merge November 11, 2023 16:09

shawkins reviewed Nov 13, 2023

View reviewed changes

shawkins self-requested a review November 13, 2023 21:29

shawkins mentioned this pull request Nov 13, 2023

fix: check operand StatefulSet is ready before creating job #24720

Merged

Piotr Godowski and others added 3 commits November 14, 2023 09:51

Merge branch 'fix/realm-import-prereq-check' of https://github.com/pg…

605a1a3

…odowski/keycloak into fix/realm-import-prereq-check

Test realm import fails when Keycloak is not running

6da3df9

Signed-off-by: Piotr Godowski <piotr.godowski@pl.ibm.com>

Merge branch 'release/22.0' into fix/realm-import-prereq-check

a5a7fcd

shawkins approved these changes Nov 14, 2023

View reviewed changes

ghost mentioned this pull request Nov 14, 2023

Flaky test: org.keycloak.testsuite.ui.account2.MyResourcesTest#shouldAllowRequestToShare #22853

Closed

ghost reviewed Nov 14, 2023

View reviewed changes

vmuzikar approved these changes Nov 15, 2023

View reviewed changes

vmuzikar enabled auto-merge (squash) November 15, 2023 07:57

vmuzikar linked an issue Nov 15, 2023 that may be closed by this pull request

Keycloak operator 22.0.z created RealmImport Job withouth checking Keycloak operand health first #24526

Closed

1 task

ahus1 disabled auto-merge November 27, 2023 19:16

ahus1 merged commit 002f1a7 into keycloak:release/22.0 Nov 27, 2023
62 checks passed

stianst mentioned this pull request May 6, 2024

Ignore github links when checking links #29291

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check operand StatefulSet is ready for realm import Job to run #24534

check operand StatefulSet is ready for realm import Job to run #24534

pgodowski commented Nov 3, 2023

shawkins left a comment

pgodowski commented Nov 5, 2023

vmuzikar left a comment

pgodowski commented Nov 6, 2023

pgodowski commented Nov 7, 2023 •

edited

Loading

shawkins commented Nov 9, 2023

vmuzikar left a comment •

edited

Loading

pgodowski commented Nov 9, 2023

shawkins left a comment

pgodowski commented Nov 10, 2023

ghost commented Nov 10, 2023

ghost left a comment

shawkins commented Nov 10, 2023

shawkins Nov 13, 2023

pgodowski Nov 14, 2023

shawkins commented Nov 13, 2023

pgodowski commented Nov 14, 2023

shawkins left a comment

ghost left a comment

ghost commented Nov 14, 2023

vmuzikar commented Nov 15, 2023

Jamstah commented Nov 23, 2023 •

edited

Loading

pgodowski commented Nov 23, 2023

Jamstah commented Nov 23, 2023

cypress bot commented Nov 27, 2023

check operand StatefulSet is ready for realm import Job to run #24534

check operand StatefulSet is ready for realm import Job to run #24534

Conversation

pgodowski commented Nov 3, 2023

shawkins left a comment

Choose a reason for hiding this comment

pgodowski commented Nov 5, 2023

vmuzikar left a comment

Choose a reason for hiding this comment

pgodowski commented Nov 6, 2023

pgodowski commented Nov 7, 2023 • edited Loading

shawkins commented Nov 9, 2023

vmuzikar left a comment • edited Loading

Choose a reason for hiding this comment

pgodowski commented Nov 9, 2023

shawkins left a comment

Choose a reason for hiding this comment

pgodowski commented Nov 10, 2023

ghost commented Nov 10, 2023

Unreported flaky test detected

org.keycloak.testsuite.ui.account2.WelcomeScreenTest#accountSecurityTest

ghost left a comment

Choose a reason for hiding this comment

shawkins commented Nov 10, 2023

shawkins Nov 13, 2023

Choose a reason for hiding this comment

pgodowski Nov 14, 2023

Choose a reason for hiding this comment

shawkins commented Nov 13, 2023

pgodowski commented Nov 14, 2023

shawkins left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost commented Nov 14, 2023

Unreported flaky test detected

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginSuccessWithCRLSignedWithIntermediateCA3FromTruststore

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromFile

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromHttp

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithInvalidSignatureCRL

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginWithMultipleRevocationLists

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginSuccessWithEmptyRevocationListFromFile

vmuzikar commented Nov 15, 2023

Jamstah commented Nov 23, 2023 • edited Loading

pgodowski commented Nov 23, 2023

Jamstah commented Nov 23, 2023

cypress bot commented Nov 27, 2023

1 failed and 2 flaky tests on run #9989 ↗︎

Review all test suite changes for PR #24534 ↗︎

pgodowski commented Nov 7, 2023 •

edited

Loading

vmuzikar left a comment •

edited

Loading

Jamstah commented Nov 23, 2023 •

edited

Loading