Skip to content

Ineffectual assignment of the envVars for connection pooler #2165

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? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jan 12, 2023
Merged

Ineffectual assignment of the envVars for connection pooler #2165

merged 2 commits into from
Jan 12, 2023

Conversation

dmvolod
Copy link
Contributor

@dmvolod dmvolod commented Jan 9, 2023

Ineffectual assignment of the envVars for connection pooler if spec.TLS is specified.
@FxKu please have a look at this bug and try to fit it into the 1.9 if possible
Test case can be added, but this case should be covered by the linter

FxKu
FxKu reviewed Jan 10, 2023
View reviewed changes
hack/update-codegen.sh
@@ -18,7 +18,8 @@ trap "cleanup" EXIT SIGINT
bash "${CODEGEN_PKG}/generate-groups.sh" all \
"${OPERATOR_PACKAGE_ROOT}/pkg/generated" "${OPERATOR_PACKAGE_ROOT}/pkg/apis" \
"acid.zalan.do:v1 zalando.org:v1" \
--go-header-file "${SCRIPT_ROOT}"/hack/custom-boilerplate.go.txt
--go-header-file "${SCRIPT_ROOT}"/hack/custom-boilerplate.go.txt \
-o ./
Copy link
Member

Choose a reason for hiding this comment

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

unrelated change. What is it for?

Copy link
Contributor Author

@dmvolod dmvolod Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is fix when GOPATH installed explicitly (for example in GoLand IDE and multiple golang version installed, but not happens for the single and system golang version) code generation will be targeted to the GOPATH\src instead of the root project path.

@FxKu FxKu added this to the 1.9 milestone Jan 10, 2023
@FxKu
Copy link
Member

FxKu commented Jan 10, 2023 •
edited
Loading

Thanks for raising @dmvolod. We wanted to add a unit test today and would have stumbled over it, I guess 😃
@jeremie-seguin fyi

@dmvolod
Copy link
Contributor Author

dmvolod commented Jan 11, 2023

We wanted to add a unit test today and would have stumbled over it

Seems to TestConnectionPoolerPodSpec need to be refactoring a bit and not ready for easy env vars validation. Probably it can be postponed to next release, as current situation can be handled by the linting step in the future, but complex test solution for env vars need much more time.
@FxKu wdyt about it?

@FxKu
Copy link
Member

FxKu commented Jan 12, 2023

👍

1 similar comment
@jopadi
Copy link
Member

jopadi commented Jan 12, 2023

👍

@FxKu FxKu merged commit ce1fee8 into zalando:master Jan 12, 2023
@FxKu
Copy link
Member

FxKu commented Jan 12, 2023

Thanks @dmvolod. Yeah we also noticed that pooler unit test need more work than what @jopadi and me expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FxKu FxKu FxKu left review comments

@sdudoladov sdudoladov Awaiting requested review from sdudoladov sdudoladov is a code owner

@Jan-M Jan-M Awaiting requested review from Jan-M Jan-M is a code owner

@jopadi jopadi Awaiting requested review from jopadi jopadi is a code owner

@idanovinda idanovinda Awaiting requested review from idanovinda idanovinda is a code owner

@hughcapet hughcapet Awaiting requested review from hughcapet hughcapet is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.9
Development

Successfully merging this pull request may close these issues.
3 participants
@dmvolod @FxKu @jopadi