Align the message with master plugin restoration logic

k8snetworkplumbingwg / multus-cni

A CNI meta-plugin for multi-homed pods in Kubernetes

Apache License 2.0

2.36k stars 585 forks source link

Align the message with master plugin restoration logic #1190

Closed kichankwon closed 1 month ago

kichankwon commented 10 months ago

thin_entrypoint prints that it allows 45 seconds for its restoration, but it waits until "masterConfigFilePath" exists. So, adjust the message to align with this logic.

s1061123 commented 10 months ago

Thank you for the PR. Good catch that mismatch between comment and actual code. But I would like to know why you fix to 45sec wait, not wait forever. If we cannot find master config, then your change makes the pod quit and launch it again...

How about to change to wait forever until the master config found?

kichankwon commented 9 months ago

Thank you for the PR. Good catch that mismatch between comment and actual code. But I would like to know why you fix to 45sec wait, not wait forever. If we cannot find master config, then your change makes the pod quit and launch it again...

How about to change to wait forever until the master config found?

I fixed to 45 seconds wait because..

I thought there's a mistake in the message or logic
And I found that old shell script(images/entrypoint.sh) waits 45 seconds just before it got removed
- https://github.com/k8snetworkplumbingwg/multus-cni/blob/c129e72779ce2d97f848f5409f6b289846e4b7d8/images/entrypoint.sh#L457
So, I decided to fix the logic

But, I agree with you. So, I'm going to fix the message not the logic.

coveralls commented 9 months ago

coverage: 63.211% (-0.3%) from 63.468% when pulling 4f7dce6f37d6539371b6f9906e3a109fcde3c2bc on kichankwon:set_timeout into acfbd42719545844248ecc35cc93d0ce367f87e5 on k8snetworkplumbingwg:master.

kichankwon commented 8 months ago

@s1061123 Could you check this PR?

kichankwon commented 6 months ago

@s1061123 Can I ping you for a review?

github-actions[bot] commented 2 months ago

This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.