Skip to content
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

Bug 1795696: templates: etcd-member: setup environment variables needed for easy etcdctl execution #1429

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,21 @@ contents:
mountPath: /var/lib/etcd/
- name: conf
mountPath: /etc/etcd/
lifecycle:
postStart:
exec:
command:
- /bin/sh
- -c
- echo 'export ETCDCTL_CACERT=/etc/ssl/etcd/ca.crt ETCDCTL_CERT=$(find /etc/ssl/ -name *peer*crt) ETCDCTL_KEY=$(find /etc/ssl/ -name *peer*key)' >> /root/.profile
Copy link
Member

Choose a reason for hiding this comment

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

I really don't think we should have starting any containers affect host content like this by default.

The etcd recovery tools are going to be containerized anyways, so let's just document this stuff over there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The etcd recovery tools are going to be containerized in 4.5+. And it assumes cluster-etcd-operator to be available, which is not available prior to 4.4. That means, our current customers of 4.2 and 4.3 (and perhaps 4.4) will not have easy execution of etcdctl.

Also .profile is current not used by anyone (that file doesn't exist at all in most pod installations). So, I do not see a reason to oppose it. If you don't like the use of .profile, I can put it in a file named .etcdctl_env.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I mistakenly thought these hooks ran on the host.


env:
- name: ETCDCTL_API
value: "3"
- name: ETCD_DATA_DIR
value: "/var/lib/etcd"
- name: ENV
value: "/root/.profile"
Copy link
Contributor Author

@retroflexer retroflexer Feb 2, 2020

Choose a reason for hiding this comment

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

In RHCOS, /bin/sh is symbolically linked to bash, but it tries to mimic the behavior of historical bourne shell.

When bash is invoked as an interactive shell with the name sh, bash looks for the variable ENV, expands its value if it is defined, and uses the expanded value as the name of a file to read and execute. Since a shell invoked as sh does not attempt to read and execute commands from any other startup files (including .profile), the --rcfile option has no effect.
(from the man pages of bash)

Copy link
Contributor

Choose a reason for hiding this comment

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

@retroflexer thanks, for the crisp explanation. I wonder if it would be a good idea to create a secret/configmap for the env variables here and then mount the configmap? I think it would be lot cleaner and admins can configure the env variable independent of static pod yaml.

/cc @hexfusion

- name: ETCD_NAME
valueFrom:
fieldRef:
Expand Down