Automatically propagate ca-certs configmap content into server and identity provider #487

tolusha · 2020-10-08T10:01:26Z

Signed-off-by: Anatolii Bazko abazko@redhat.com

Reference issue

eclipse-che/che#17918

What does this PR do

add owner to ca-certs configmap (if necessary)
update environment variables in keyckoak and server deployments to restart pods to take into account new certificates from the ca-certs configmap.

…nd identity provider Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

openshift-ci-robot · 2020-10-08T10:01:37Z

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

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

pkg/deploy/owner.go

mmorhun · 2020-10-09T08:09:01Z

pkg/deploy/identity-provider/deployment_keycloak.go

@@ -221,6 +223,10 @@ func getSpecKeycloakDeployment(
 	}

 	keycloakEnv := []corev1.EnvVar{
+		{
+			Name:  "CM_REVISIONS",


Why do we need this in the Keycloak and Che containers env? If we need to mark something, isn't labels does the work?

pkg/deploy/owner.go

davidfestal

Shouldn't we also restart the Che server when the content of this config map changes ?
Maybe it is already done and I missed it ?

davidfestal · 2020-10-09T08:41:25Z

pkg/controller/che/che_controller.go

+	if instance.Spec.Server.ServerTrustStoreConfigMapName != "" {
+		trustStoreConfigMap, _ := deploy.GetClusterConfigMap(instance.Spec.Server.ServerTrustStoreConfigMapName, instance.Namespace, clusterAPI.Client)
+		if trustStoreConfigMap != nil {
+			// trustStoreConfigMap might be created by user, to detect changes we have to add the owner


Why ? Isn't the main method run regularly anyway ?
Not sure what the requirement for an owner will mean when we select all the config maps that have a given label.

tolusha · 2020-10-09T08:54:47Z

@davidfestal
We ensure that Che server is updated as well.
https://github.com/eclipse/che-operator/pull/487/files#diff-d9445e614f0d728fd140771b0fd29c50R842

davidfestal · 2020-10-09T08:55:45Z

@davidfestal
Here is we ensure that Che server is updated as well.
https://github.com/eclipse/che-operator/pull/487/files#diff-d9445e614f0d728fd140771b0fd29c50R842

Oh, I see. Sorry I missed it.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

pkg/controller/che/che_controller.go

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha · 2020-10-09T12:32:25Z

It is turned out that changing annotations does not restart pods.
I've to return to env variables.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha · 2020-10-09T12:45:11Z

@davidfestal
It is ready to review again

pkg/deploy/identity-provider/identity_provider.go

pkg/deploy/server/deployment_che.go

pkg/deploy/identity-provider/deployment_keycloak.go

pkg/controller/che/che_controller.go

pkg/deploy/identity-provider/deployment_keycloak.go

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha · 2020-10-12T10:29:41Z

@davidfestal
All comments are addressed.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

openshift-ci-robot · 2020-10-12T12:38:23Z

@tolusha: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/v5-che-operator-update	`60be304`	link	`/test v5-che-operator-update`

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.

davidfestal

Great that now we won't have to rollout any POD manually after adding certificates.

openshift-ci-robot · 2020-10-13T08:40:04Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: davidfestal, tolusha
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

tolusha · 2020-10-13T08:48:12Z

@davidfestal
I am going to port changes into 7.20.x branch to have this feature done for crw 2.5

…entity provider (#487) * Automatically propagate serverTrustStoreConfigMap context to server and identity provider Signed-off-by: Anatolii Bazko <abazko@redhat.com>

…entity provider (#487) (#493) * Automatically propagate serverTrustStoreConfigMap context to server and identity provider Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha added 2 commits October 8, 2020 13:00

Automatically propagate serverTrustStoreConfigMap context to server a…

37c5dda

…nd identity provider Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Add missed files

f249f08

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

openshift-ci-robot added the do-not-merge/work-in-progress label Oct 8, 2020

tolusha mentioned this pull request Oct 8, 2020

Che Deploy Sprint #191 eclipse-che/che#18011

Closed

50 tasks

tolusha added 6 commits October 9, 2020 09:32

Fixes

da574bb

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fixes

f269994

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fix

1e2d175

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fix

118ce6c

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fix

f3da3ab

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fixes

04efd68

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha marked this pull request as ready for review October 9, 2020 06:44

tolusha requested review from davidfestal, nickboldt and vparfonov as code owners October 9, 2020 06:44

openshift-ci-robot removed the do-not-merge/work-in-progress label Oct 9, 2020

tolusha changed the title ~~Automatically propagate serverTrustStoreConfigMap context to server a…~~ Automatically propagate ca-certs configmap content into server and identity provider Oct 9, 2020

tolusha requested review from AndrienkoAleksandr and mmorhun October 9, 2020 06:49

AndrienkoAleksandr reviewed Oct 9, 2020

View reviewed changes

pkg/deploy/owner.go Outdated Show resolved Hide resolved

mmorhun reviewed Oct 9, 2020

View reviewed changes

AndrienkoAleksandr reviewed Oct 9, 2020

View reviewed changes

pkg/deploy/owner.go Outdated Show resolved Hide resolved

davidfestal reviewed Oct 9, 2020

View reviewed changes

Fixes

3b73704

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

AndrienkoAleksandr reviewed Oct 9, 2020

View reviewed changes

pkg/controller/che/che_controller.go Show resolved Hide resolved

tolusha added 2 commits October 9, 2020 15:23

Move resources version to annotations

36e3abe

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Fix

5206038

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

tolusha added 2 commits October 9, 2020 15:37

Fiex

f6db2a6

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Revert cr.yaml

9cf64d1

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

davidfestal reviewed Oct 12, 2020

View reviewed changes

Fix remarks

67e7e54

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

revert go.mod go.sum

60be304

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

davidfestal approved these changes Oct 13, 2020

View reviewed changes

openshift-ci-robot assigned davidfestal Oct 13, 2020

openshift-ci-robot added the lgtm label Oct 13, 2020

tolusha merged commit 47a01d7 into master Oct 13, 2020

tolusha deleted the updatedeploymentswhencacertupdated branch October 13, 2020 11:04

This was referenced Oct 13, 2020

Automatically propagate ca-certs configmap content into server and identity provider #492

Closed

Automatically propagate ca-certs configmap content into server and identity provider #493

Merged

l0rd mentioned this pull request Oct 15, 2020

Update the documentation about importing external certificates eclipse-che/che-docs#1598

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically propagate ca-certs configmap content into server and identity provider #487

Automatically propagate ca-certs configmap content into server and identity provider #487

tolusha commented Oct 8, 2020 •

edited

Loading

openshift-ci-robot commented Oct 8, 2020

mmorhun Oct 9, 2020

tolusha Oct 9, 2020

davidfestal left a comment

davidfestal Oct 9, 2020

tolusha commented Oct 9, 2020 •

edited

Loading

davidfestal commented Oct 9, 2020

tolusha commented Oct 9, 2020

tolusha commented Oct 9, 2020

tolusha commented Oct 12, 2020

openshift-ci-robot commented Oct 12, 2020

davidfestal left a comment

openshift-ci-robot commented Oct 13, 2020

tolusha commented Oct 13, 2020

Automatically propagate ca-certs configmap content into server and identity provider #487

Automatically propagate ca-certs configmap content into server and identity provider #487

Conversation

tolusha commented Oct 8, 2020 • edited Loading

Reference issue

What does this PR do

openshift-ci-robot commented Oct 8, 2020

mmorhun Oct 9, 2020

Choose a reason for hiding this comment

tolusha Oct 9, 2020

Choose a reason for hiding this comment

davidfestal left a comment

Choose a reason for hiding this comment

davidfestal Oct 9, 2020

Choose a reason for hiding this comment

tolusha commented Oct 9, 2020 • edited Loading

davidfestal commented Oct 9, 2020

tolusha commented Oct 9, 2020

tolusha commented Oct 9, 2020

tolusha commented Oct 12, 2020

openshift-ci-robot commented Oct 12, 2020

davidfestal left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 13, 2020

tolusha commented Oct 13, 2020

tolusha commented Oct 8, 2020 •

edited

Loading

tolusha commented Oct 9, 2020 •

edited

Loading