Add System's USER_SESSION_TTL configurability #618
Conversation
|
Pending to do some verifications in a real deployment but let's review it first and if it looks ok I'll proceed to do the verifications |
|
Wow, a lot of changes just for one new option. |
| if err != nil { | ||
| return reconcile.Result{}, err | ||
| } | ||
| baseAPIManagerLogicReconciler := NewBaseAPIManagerLogicReconciler(u.BaseReconciler, u.apiManager) |
There was a problem hiding this comment.
Not related to this PR, maybe for another PR: what about embedding BaseAPIManagerLogicReconciler in UpgradeApiManager type?
There was a problem hiding this comment.
👍 I'll take a look at it and open a new PR in that case
pkg/3scale/amp/operator/upgrade.go
Outdated
| @@ -638,11 +688,11 @@ func (u *UpgradeApiManager) upgradeSidekiqBackendRouteEnv(desired *appsv1.Deploy | |||
| return reconcile.Result{Requeue: update}, nil | |||
| } | |||
|
|
|||
| func ensureBackendRouteEnvVar(desired v1.EnvVar, existingEnvVars *[]v1.EnvVar) bool { | |||
| func ensureEnvVar(desired v1.EnvVar, existingEnvVars *[]v1.EnvVar) bool { | |||
There was a problem hiding this comment.
This method belongs to pkg/helper/envvarutils.go
There was a problem hiding this comment.
👍 I'll add it there
I thought about that too but the upgrade is needed because the environment variable needs to be added to the deployment, during regular reconciliation that part is not reconciled, and the value has to be added to the secret because the environment variable references it. |
e41d15e to
6fba968
Compare
6fba968 to
05fd286
Compare
|
Code Climate has analyzed commit 05fd286 and detected 5 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
/retest |
1 similar comment
|
/retest |
|
/retest |
1 similar comment
|
/retest |
This PR exposes the
USER_SESSION_TTLSystem's setting.Related issue:
It exposes this setting as an attribute in the
system-appsecret as usual.This is the behavior:
Secret side:
""stringSystem pod side
""then USER_SESSION_TTL on system side will be the default at system levelAn upgrade procedure has been implemented too to make sure the secret entry is created and that the system-app pod containers reference this secret entry.
The change has also been applied to templates. This change will require an upgrade procedure for templates too.
Pending:
[ ] Functionality verification in a real deployment, in both a clean environment and an upgrade