Skip to content

Add Solr ZK Connection Options#456

Merged
gerlowskija merged 7 commits intoapache:mainfrom
HoustonPutman:zk-ssl
Jul 27, 2022
Merged

Add Solr ZK Connection Options#456
gerlowskija merged 7 commits intoapache:mainfrom
HoustonPutman:zk-ssl

Conversation

@HoustonPutman
Copy link
Contributor

@HoustonPutman HoustonPutman commented Jul 8, 2022

Resolves #435

This is a bit broader than the Issue, but should be easier for users to setup.

Still to do:

  • Finish tests
  • Add helm options
  • Add documentation & changelog entry

@gerlowskija
Copy link
Contributor

Alright, I've taken a crack at the remaining TODOs (tests, helm chart exposure, and docs). This should be ready for review if anyone gets a chance!

testGenericPodEnvVariablesWithGomega(g, expectedEnvVars, foundEnvVars, "SOLR_OPTS", resolveOffset(additionalOffset))
}

func testMetricsPodEnvVariables(expectedEnvVars map[string]string, foundEnvVars []corev1.EnvVar, additionalOffset ...int) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These methods were all able to be merged, as the only thing differentiating them previously was the env-var that they required to be last.

Copy link
Contributor Author

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

I can't approve this because it's my PR, but:

Absolutely beautiful, especially the new checks for EnvVar dependencies.

LGTM to merge!

@gerlowskija
Copy link
Contributor

Hah, but it lets me Approve despite having the latest commits on the branch 🤷

Thanks for the review Houston!

Bit more manual testing but plan to merge this afternoon.

@gerlowskija gerlowskija merged commit 8821dc7 into apache:main Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow option to pass ZKCLI_JVM_FLAGS to cloud-scripts zkcli.sh

2 participants

Comments