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

cp configmap elements over to config volume as configmap vol is RO #163

Merged
merged 7 commits into from
Mar 24, 2018

Conversation

lamdor
Copy link
Contributor

@lamdor lamdor commented Mar 22, 2018

I ran into an issue when deploying this onto our Tectonic kubernetes cluster. It turns out that the ConfigMap volumes are mounted read-only. I'm unsure if this is standard kubernetes behavior, but minikube does not do the same thing; it allows modification of the configmap files. Glancing at kubernetes code, it appears it should be mounted read-only.

So to fix this, I made a new emptyDir volume for where the config should go and then make the init containers copy the configmap files before they modify them.

@pfridberg
Copy link

I ran into the same problem as well. This fix seems to be working as intended on our cluster.

@solsson solsson added this to the 4.0 milestone Mar 23, 2018
@solsson
Copy link
Contributor

solsson commented Mar 23, 2018

Fixes #162. I will give it a test before I merge, in a day or two.

@solsson
Copy link
Contributor

solsson commented Mar 23, 2018

@rubbish I've pushed two commits, one to apply the fix for ephemeral zoo and one to correct an inconsistency observed by in @clintfred in #162 (comment). I've verified these in a test cluster. If you're ok with them I will merge.

Copy link

@clintfred clintfred left a comment

Choose a reason for hiding this comment

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

LGTM

@lamdor
Copy link
Contributor Author

lamdor commented Mar 24, 2018

LGTM too. thanks @solsson

README.md Outdated
@@ -28,6 +28,7 @@ If you begin to rely on this kafka setup we recommend you fork, for example to e

| tag | k8s ≥ | highlights |
| ----- | ------ | ---------- |
| master | 1.9.4 | Required for read-only ConfigMaps, k8s 1.9.4+ |
Copy link
Contributor Author

@lamdor lamdor Mar 24, 2018

Choose a reason for hiding this comment

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

Quick note that the readonly configmap volume fix was backported and released as 1.8.9 and 1.7.14 too so each of those need master as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks again @rubbish. I have apparently not been paying enough attention. I'll merge this now.

@solsson solsson merged commit 3846f98 into Yolean:master Mar 24, 2018
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.

4 participants