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

configurable crictl endpoint #97

Merged
merged 2 commits into from
Jun 23, 2022
Merged

configurable crictl endpoint #97

merged 2 commits into from
Jun 23, 2022

Conversation

tanmaykm
Copy link
Contributor

When DEPLOY_CRIO_CONFIG is true, both image-endpoint and runtime-endpoint are generated as hard coded values here. This PR is to make them optionally configurable. To configure it, daemonset.crioEndpoint can now be set in values.yaml, which translates to the CRIO_ENDPOINT environment variable in the daemonset.

Found this useful on setups that have dockershim. The default is still set to unix:///run/containerd/containerd.sock, so this should be backwards compatible.

When `DEPLOY_CRIO_CONFIG` is `true`, both `image-endpoint` and `runtime-endpoint` are generated as hard coded values [here](https://github.com/IBM/core-dump-handler/blob/a67034ad7e807df956947b74b770a978235bf27b/core-dump-agent/src/main.rs#L422). This PR is to make them optionally configurable. To configure it, `daemonset.crioEndpoint` can now be set in `values.yaml`, which translates to the `CRIO_ENDPOINT` environment variable in the daemonset.

Found this useful on setups that have dockershim. The default is still set to `unix:///run/containerd/containerd.sock`, so this should be backwards compatible.
@No9
Copy link
Collaborator

No9 commented Jun 23, 2022

Really appreciate the contribution.
With k8s withdrawing docker support and the tested cloud providers using the crio defaults I wasn't sure that an alternative location would be needed so I left it open but unimplemented.
As you seem to have a need I'm happy to merge once the rustfmt nit is fixed.
Thanks again for the contribution.

@tanmaykm
Copy link
Contributor Author

Thanks @No9 for accepting this PR!
I have pushed 3c21b08 which will hopefully address rustfmt requirement

@No9 No9 merged commit 8b4a8a2 into IBM:main Jun 23, 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.

2 participants