8000 MGDAPI-5152 - remove duplicate FILE_UPLOAD_STORAGE env var by adam-cattermole · Pull Request #811 · 3scale/3scale-operator · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

MGDAPI-5152 - remove duplicate FILE_UPLOAD_STORAGE env var #811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? 8000 Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Apr 14, 2023
Merged

MGDAPI-5152 - remove duplicate FILE_UPLOAD_STORAGE env var #811

merged 4 commits into from
Apr 14, 2023

Conversation

adam-cattermole
Copy link
Contributor
@adam-cattermole adam-cattermole commented Apr 11, 2023 •
edited
Loading

FILE_UPLOAD_STORAGE env var is added as part of the config map, and added separately after resulting in duplicate env vars on some DeploymentConfigs.

On v4.11 when updating any of the deployment configs a new replication controller is created and there are warnings about a duplicate FILE_UPLOAD_STORAGE env var in the deploy logs. However on v4.13 these no longer show as warnings but as errors and the rollout is marked as failed. This duplicate env var is present as it is added from the base config map and then appended again.

v4.11

--> pre: Running hook pod ...
NOTE: Gem::Specification#has_rdoc= is deprecated with no replacement. It will be removed on or after 2018-12-01.
Gem::Specification#has_rdoc= called from /opt/deps/rubygems/github.com/3scale/prawn/prawn-external-gitcommit-88aead0d97e230d08e9f51a18711d01317965bfe/app/prawn-core.gemspec:36.
fatal: not a git repository (or any of the parent directories): .git
Removing :environment prerequisite from db:create
I, [2023-03-31T10:45:53.839087 #1] INFO -- : ActiveMerchant MODE set to 'production'
W, [2023-03-31T10:45:53.854215 #1] WARN -- [Bugsnag]: No valid API key has been set, notifications will not be sent
I, [2023-03-31T10:45:54.026795 #1] INFO -- : [Core] Using http://backend-listener:3000/internal/ as URL
OpenIdAuthentication.store is nil. Using in-memory store.
Creating scope :admins. Overwriting existing method User.admins.
Creating scope :by_name. Overwriting existing method Cinstance.by_name.
[core] non-native log levels verbose, notice, critical emulated using UNKNOWN severity
Backend Internal API version 3.4.3 status: ok
Connected to mysql2://root@system-mysql/system
Connected to redis://system-redis:6379/1
--> pre: Success
W0331 10:45:59.726113 1 warnings.go:70] would violate PodSecurity "restricted:v1.24": allowPrivilegeEscalation != false (containers "system-master", "system-provider", "system-developer" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (containers "system-master", "system-provider", "system-developer" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
W0331 10:45:59.754015 1 warnings.go:70] would violate PodSecurity "restricted:v1.24": allowPrivilegeEscalation != false (containers "system-master", "system-provider", "system-developer" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (containers "system-master", "system-provider", "system-developer" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
--> Scaling up system-app-2 from 0 to 1, scaling down system-app-1 from 1 to 0 (keep 1 pods available, don't exceed 2 pods)
Scaling system-app-2 up to 1
W0331 10:45:59.764889 1 warnings.go:70] spec.template.spec.containers[0].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
W0331 10:45:59.764905 1 warnings.go:70] spec.template.spec.containers[1].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
W0331 10:45:59.764909 1 warnings.go:70] spec.template.spec.containers[2].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
Scaling system-app-1 down to 0
W0331 10:47:05.746333 1 warnings.go:70] spec.template.spec.containers[0].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
W0331 10:47:05.746347 1 warnings.go:70] spec.template.spec.containers[1].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
W0331 10:47:05.746351 1 warnings.go:70] spec.template.spec.containers[2].env[47].name: duplicate name "FILE_UPLOAD_STORAGE"
W0331 10:47:06.778809 1 warnings.go:70] would violate PodSecurity "restricted:v1.24": allowPrivilegeEscalation != false (containers "system-master", "system-provider", "system-developer" must set securityCon
8000
text.allowPrivilegeEscalation=false), unrestricted capabilities (containers "system-master", "system-provider", "system-developer" must set securityContext.capabilities.drop=["ALL"]), runAsNonRoot != true (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.runAsNonRoot=true), seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
--> post: Running hook pod ...
NOTE: Gem::Specification#has_rdoc= is deprecated with no replacement. It will be removed on or after 2018-12-01.
Gem::Specification#has_rdoc= called from /opt/deps/rubygems/github.com/3scale/prawn/prawn-external-gitcommit-88aead0d97e230d08e9f51a18711d01317965bfe/app/prawn-core.gemspec:36.
fatal: not a git repository (or any of the parent directories): .git
Removing :environment prerequisite from db:create
I, [2023-03-31T10:47:18.526897 #1] INFO -- : ActiveMerchant MODE set to 'production'
W, [2023-03-31T10:47:18.541539 #1] WARN -- [Bugsnag]: No valid API key has been set, notifications will not be sent
I, [2023-03-31T10:47:18.711720 #1] INFO -- : [Core] Using http://backend-listener:3000/internal/ as URL
OpenIdAuthentication.store is nil. Using in-memory store.
Creating scope :admins. Overwriting existing method User.admins.
Creating scope :by_name. Overwriting existing method Cinstance.by_name.
[core] non-native log levels verbose, notice, critical emulated using UNKNOWN severity
Backend Internal API version 3.4.3 status: ok
Connected to mysql2://root@system-mysql/system
Connected to redis://system-redis:6379/1
Enqueued BackendStorageRewriteWorker#5b9bbe78ad70d110a38d8a61 with args: [1]
Enqueued BackendStorageRewriteWorker#318f39ded0f3d1e70cca4165 with args: [2]
Enqueued 2 accounts for rewrite
--> post: Success
--> Success

v4.13

--> pre: Running hook pod ...
NOTE: Gem::Specification#has_rdoc= is deprecated with no replacement. It will be removed on or after 2018-12-01.
Gem::Specification#has_rdoc= called from /opt/deps/rubygems/github.com/3scale/prawn/prawn-external-gitcommit-88aead0d97e230d08e9f51a18711d01317965bfe/app/prawn-core.gemspec:36.
fatal: not a git repository (or any of the parent directories): .git
Removing :environment prerequisite from db:create
I, [2023-03-31T12:23:43.046402 #1] INFO -- : ActiveMerchant MODE set to 'production'
W, [2023-03-31T12:23:43.061022 #1] WARN -- [Bugsnag]: No valid API key has been set, notifications will not be sent
I, [2023-03-31T12:23:43.219970 #1] INFO -- : [Core] Using http://backend-listener:3000/internal/ as URL
OpenIdAuthentication.store is nil. Using in-memory store.
Creating scope :admins. Overwriting existing method User.admins.
Creating scope :by_name. Overwriting existing method Cinstance.by_name.
[core] non-native log levels verbose, notice, critical emulated using UNKNOWN severity
Backend Internal API version 3.4.3 status: ok
Connected to mysql2://root@system-mysql/system
Connected to redis://system-redis:6379/1
--> pre: Success
error: couldn't assign source annotation to deployment system-app-2: failed to create manager for existing fields: failed to convert new object (3scale-operator/system-app-2; /v1, Kind=ReplicationController) to smd typed: errors:
.spec.template.spec.containers[name="system-master"].env: duplicate entries for key [name="FILE_UPLOAD_STORAGE"]
.spec.template.spec.containers[name="system-provider"].env: duplicate entries for key [name="FILE_UPLOAD_STORAGE"]
.spec.template.spec.containers[name="system-developer"].env: duplicate entries for key [name="FILE_UPLOAD_STORAGE"]

With the changes in this PR we no longer have the duplicate entry and the deployment config can be updated successfully on v4.13:

--> pre: Running hook pod ...
Removing :environment prerequisite from db:create
I, [2023-03-31T13:36:08.725595 #1] INFO -- : ActiveMerchant MODE set to 'production'
W, [2023-03-31T13:36:08.743611 #1] WARN -- [Bugsnag]: No valid API key has been set, notifications will not be sent
I, [2023-03-31T13:36:08.918910 #1] INFO -- : [Core] Using http://backend-listener:3000/internal/ as URL
OpenIdAuthentication.store is nil. Using in-memory store.
Creating scope :admins. Overwriting existing method User.admins.
Creating scope :by_name. Overwriting existing method Cinstance.by_name.
[core] non-native log levels verbose, notice, critical emulated using UNKNOWN severity
Backend Internal API version 3.4.3 status: ok
Connected to mysql2://root@system-mysql/system
Connected to redis://system-redis:6379/1
--> pre: Success
W0331 13:36:13.598613 1 warnings.go:70] would violate PodSecurity "restricted:latest": seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
W0331 13:36:13.641820 1 warnings.go:70] would violate PodSecurity "restricted:latest": seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
--> Scaling up system-app-2 from 0 to 1, scaling down system-app-1 from 1 to 0 (keep 1 pods available, don't exceed 2 pods)
Scaling system-app-2 up to 1
Scaling system-app-1 down to 0
W0331 13:37:15.742399 1 warnings.go:70] would violate PodSecurity "restricted:latest": seccompProfile (pod or containers "system-master", "system-provider", "system-developer" must set securityContext.seccompProfile.type to "RuntimeDefault" or "Localhost")
--> post: Running hook pod ...
Removing :environment prerequisite from db:create
I, [2023-03-31T13:37:24.713843 #1] INFO -- : ActiveMerchant MODE set to 'production'
W, [2023-03-31T13:37:24.731184 #1] WARN -- [Bugsnag]: No valid API key has been set, notifications will not be sent
I, [2023-03-31T13:37:24.907571 #1] INFO -- : [Core] Using http://backend-listener:3000/internal/ as URL
OpenIdAuthentication.store is nil. Using in-memory store.
Creating scope :admins. Overwriting existing method User.admins.
Creating scope :by_name. Overwriting existing method Cinstance.by_name.
[core] non-native log levels verbose, notice, critical emulated using UNKNOWN severity
Backend Internal API version 3.4.3 status: ok
Connected to mysql2://root@system-mysql/system
Connected to redis://system-redis:6379/1
Enqueued BackendStorageRewriteWorker#ec164faac0d81388c0fdd09f with args: [1]
Enqueued BackendStorageRewriteWorker#23d2aaff28bd257b1638c692 with args: [2]
Enqueued 2 accounts for rewrite
--> post: Success
--> Success

WIP as I have not verified the upgrade scenario

@adam-cattermole adam-cattermole requested a review from a team as a code owner April 11, 2023 13:47
@openshift-ci
Copy link
openshift-ci bot commented Apr 11, 2023

Hi @adam-cattermole. Thanks for your PR.

I'm waiting for a 3scale member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@adam-cattermole adam-cattermole mentioned this pull request Apr 11, 2023
Merged
@eguzki
Copy link
Member
eguzki commented Apr 11, 2023

/ok-to-test

eguzki
eguzki previously approved these changes Apr 11, 2023
View reviewed changes
@adam-cattermole
Copy link
Contributor Author
adam-cattermole commented Apr 11, 2023 •
edited
Loading

I've just managed to test these changes for upgrade and whilst they work for fresh install it won't work on upgrade. The env vars aren't updated when reconciling the DCs when they exist with duplicates already.

@eguzki
Copy link
Member
eguzki commented Apr 11, 2023

I've just managed to test these changes for upgrade and whilst they work for fresh install it won't work on upgrade. We're going to have to check the DCs env vars and explicitly remove one of the duplicates.

right!

We need to clean the DC first.

Does that mean that 3scale needs to be upgraded before upgrading OCP to 4.13?

@adam-cattermole
Copy link
Contributor Author

Does that mean that 3scale needs to be upgraded before upgrading OCP to 4.13?

I'm unsure on this, as far as I can tell the existing replication controller continues to run fine, but any subsequent rollouts appear to fail. As long as the DC is corrected then a new rollout will be triggered that will fix it as far as I can tell.

@eguzki eguzki dismissed their stale review April 11, 2023 15:44

upgrade procedure is missing

@eguzki
Copy link
Member
eguzki commented Apr 13, 2023

This is interesting @adam-cattermole. I would like to have progress on this issue. Let me know if you need advise to implement the upgrade procedure. I am more than happy to jump to a call and pair review it.

@adam-cattermole
Copy link
Contributor Author

@eguzki as @KevFan has taken over the RHOAM v4.13 ticket I've handed this PR over to him to progress the fix for upgrade and provided some details on the issue. I'll be available to help/verify if needed

KevFa 8000 n
KevFan reviewed Apr 13, 2023
View reviewed changes
@KevFan KevFan force-pushed the MGDAPI-5152-duplicate-env branch from 44fc70b to 28cd16a Compare April 13, 2023 13:31
@KevFan KevFan force-pushed the MGDAPI-5152-duplicate-env branch from 28cd16a to afe7265 Compare April 13, 2023 13:33
@KevFan KevFan changed the title [WIP] MGDAPI-5152 - remove duplicate FILE_UPLOAD_STORAGE env var MGDAPI-5152 - remove duplicate FILE_UPLOAD_STORAGE env var Apr 13, 2023
@KevFan
Copy link
Contributor
KevFan commented Apr 13, 2023

@eguzki Latest commit should fix and prevent duplicate env vars on upgrade for system and sidekiq if you can review whenever you get the chance 🙏

eguzki
eguzki approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member
@eguzki eguzki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏅

@KevFan
Copy link
Contributor
KevFan commented Apr 13, 2023

Pushed a commit to fix some typos from refactor

eguzki
eguzki reviewed Apr 14, 2023
View reviewed changes
@codeclimate
Copy link
codeclimate bot commented Apr 14, 2023

Code Climate has analyzed commit bfcbd98 and detected 0 issues on this pull request.

View more on Code Climate.

@eguzki
Copy link
Member
eguzki commented Apr 14, 2023

🎖️

@eguzki eguzki merged commit 362f026 into 3scale:master Apr 14, 2023
@eguzki eguzki mentioned this pull request Apr 14, 2023
Merged
eguzki added a commit that referenced this pull request Apr 17, 2023
@eguzki
Merge pull request #816 from 3scale/backport-811-MGDAPI-5152-duplicat…
08c8cdc 
…e-env

[Backport #811 MGDAPI-5152] remove duplicate env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KevFan KevFan KevFan left review comments

@eguzki eguzki eguzki approved these changes

Assignees
No one assigned
Labels
ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adam-cattermole @eguzki @KevFan
0