Skip to content

pass 6 specific env vars to make_mycnf hook#4346

Merged
sougou merged 5 commits intovitessio:masterfrom
planetscale:ds-hook-env
Nov 15, 2018
Merged

pass 6 specific env vars to make_mycnf hook#4346
sougou merged 5 commits intovitessio:masterfrom
planetscale:ds-hook-env

Conversation

@deepthi
Copy link
Copy Markdown
Collaborator

@deepthi deepthi commented Nov 7, 2018

The env variables that are being passed to make_mycnf are:
KEYSPACE
SHARD
TABLET_TYPE
TABLET_ID
TABLET_DIR

Edit: Added one more:
MYSQL_PORT

Signed-off-by: deepthi deepthi@planetscale.com

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

For consistency, let's uppercase tablet_type, etc in examples/*/vttablet-up.sh scripts. People often doctor those scripts for prod usage, and may miss this subtlety.

@sjmudd
Copy link
Copy Markdown
Contributor

sjmudd commented Nov 9, 2018

It looks like a minimum of MYSQL_PORT is also needed to enable the my.cnf file to be configured exclusively from the make_mycnf hook.

If the hook were also to have access to the exported members of the Mycnf struct then any user configuration overrides would also be seen by the hook and could be taken into account. That obviously would require more settings to be exposed.

@deepthi
Copy link
Copy Markdown
Collaborator Author

deepthi commented Nov 9, 2018

Actually the scripts are already consistent. Environment variables are uppercase and local script variables are lowercase. tablet_type is a local variable in these scripts.

For consistency, let's uppercase tablet_type, etc in examples/*/vttablet-up.sh scripts. People often doctor those scripts for prod usage, and may miss this subtlety.

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
@deepthi
Copy link
Copy Markdown
Collaborator Author

deepthi commented Nov 13, 2018

@derekperkins Can you take a look at the change to the examples? Do we want to make a similar change for the kubernetes example?

@deepthi deepthi changed the title pass 5 specific env vars to make_mycnf hook pass 6 specific env vars to make_mycnf hook Nov 13, 2018
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Let's get this through. If fixes are needed in k8s, we can do that later.

@sougou sougou merged commit 33ab9af into vitessio:master Nov 15, 2018
@sjmudd
Copy link
Copy Markdown
Contributor

sjmudd commented Nov 16, 2018

+1

@deepthi deepthi deleted the ds-hook-env branch December 5, 2018 01:07
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.

3 participants