-
Notifications
You must be signed in to change notification settings - Fork 162
K8SPSMDB-1548: allow configuring container env variables #2163
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.
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? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mayank Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for configuring custom environment variables on MongoDB containers through the env and envFrom fields in the Custom Resource Definition (CRD). The feature is version-gated to operator version 1.22.0 and above.
Key changes:
- Added
EnvandEnvFromfields to ReplsetSpec, NonVotingSpec, HiddenSpec, and MongosSpec types - Refactored the
containerfunction in statefulset.go to use a parameter struct pattern - Applied version gating (1.22.0+) to conditionally append custom environment variables
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/psmdb/v1/psmdb_types.go | Added Env and EnvFrom fields to ReplsetSpec, NonVotingSpec, HiddenSpec, and MongosSpec structs |
| pkg/apis/psmdb/v1/zz_generated.deepcopy.go | Generated DeepCopy methods for new Env and EnvFrom fields |
| pkg/psmdb/container.go | Refactored container function to accept parameters struct; added version-gated Env/EnvFrom appending |
| pkg/psmdb/statefulset.go | Updated to pass containerFnParams struct with Env and EnvFrom fields to container function |
| pkg/psmdb/mongos.go | Added version-gated Env appending for mongos containers |
| pkg/apis/psmdb/v1/psmdb_defaults.go | Minor whitespace formatting change |
| deploy/crd.yaml, deploy/bundle.yaml, deploy/cw-bundle.yaml, config/crd/bases/*.yaml, e2e-tests/version-service/conf/crd.yaml | Updated CRD definitions with env and envFrom field schemas |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
pkg/controller/perconaservermongodb/statefulset_test.go:138
- Several test cases for the cfg replica set (arbiter, non-voting, and hidden) have been commented out. These tests should either be re-enabled with updated expectations, or if they are intentionally being removed, they should be deleted rather than commented out. Commented-out code reduces code maintainability and can lead to confusion about the intended state of the test suite.
{
name: "cfg-arbiter",
cr: defaultCR.DeepCopy(),
rsName: "cfg",
component: naming.ComponentArbiter,
expectedSts: expectedSts(t, "reconcile-statefulset/cfg-arbiter.yaml"),
},
{
name: "cfg-non-voting",
cr: defaultCR.DeepCopy(),
rsName: "cfg",
component: naming.ComponentNonVoting,
expectedSts: expectedSts(t, "reconcile-statefulset/cfg-nv.yaml"),
},
{
name: "cfg-hidden",
cr: defaultCR.DeepCopy(),
rsName: "cfg",
component: naming.ComponentHidden,
expectedSts: expectedSts(t, "reconcile-statefulset/cfg-hidden.yaml"),
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
Signed-off-by: Mayank Shah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
pkg/psmdb/mongos_test.go:16
- The duplicate import of the same package with different aliases is unnecessary. Line 15 imports
apifrom the psmdb/v1 package, and line 16 importspsmdbv1from the same package. Consider removing the duplicate import on line 16 and using theapialias consistently throughout the test.
api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1"
"github.com/percona/percona-server-mongodb-operator/pkg/version"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
commit: 64e6d5c |
CHANGE DESCRIPTION
Allows configuring env variables for the
mongod,mongosandlogollectorcontainersExample:
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability