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

Ensure that configured global ConfigMap exists #804

Merged
merged 1 commit into from
Jun 26, 2021

Conversation

jcmoraisjr
Copy link
Owner

Launch is actually copying the global configmap name from the command-line straight to the lister without checking its existence. The lister will watch configmap events and will only wake up if the name matches with the one provided in the command-line, so currently a mistyped name is silently ignored.

From now the configmap name is checked in the bootstrap process and will crash if not found or if the controller doesn't have permission to read it.

This should be merged as far as v0.10, but v0.12 and older should only log error instead, due to preserve backward compatibility with the current behavior.

Launch is actually copying the global configmap name from the
command-line straight to the lister without checking its existence. The
lister will watch configmap events and will only wake up if the name
matches with the one provided in the command-line, so currently a
mistyped name is silently ignored.

From now the configmap name is checked in the bootstrap process and will
crash if not found or if the controller doesn't have permission to read
it.

This should be merged as far as v0.10, but v0.12 and older should only
log error instead, due to preserve backward compatibility with the
current behavior.
@jcmoraisjr
Copy link
Owner Author

#803

@toothbrush
Copy link
Contributor

Thank you for the quick turnaround on this one! 🙌

@jcmoraisjr jcmoraisjr merged commit ef6a26b into master Jun 26, 2021
@jcmoraisjr jcmoraisjr deleted the jm-check-configmap branch June 26, 2021 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants