whereabouts icon indicating copy to clipboard operation
whereabouts copied to clipboard

move install-cni.sh to an initContainer

Open deekue opened this issue 2 years ago • 4 comments

What this PR does / why we need it:

  • move install-cni.sh to an initContainer. separation of concerns
  • run ip-control-loop directly. daemon receives signals from k8s for graceful shutdown etc

deekue avatar Jul 17 '22 22:07 deekue

happy to add that to this PR

On Mon, Jul 18, 2022, 04:38 Miguel Duarte Barroso @.***> wrote:

@.**** requested changes on this pull request.

I think this PR improves the code base, and I agree we should be using the K8s primitives to get required stuff installed in the nodes.

... having said that, once we have this in an init container, it doesn't make sense to have that ugly bash script a long lived process. I think you should remove the SLEEP option from it, and just make it a one-shot script.

Thanks for spotting this and for following through with a PR.

— Reply to this email directly, view it on GitHub https://github.com/k8snetworkplumbingwg/whereabouts/pull/251#pullrequestreview-1041711106, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA36GX3FXNMIJDP3LT2JJR3VUU645ANCNFSM532NIHCA . You are receiving this because you authored the thread.Message ID: @.***>

deekue avatar Jul 18 '22 13:07 deekue

e2e CI failed, I'll look into it

https://github.com/k8snetworkplumbingwg/whereabouts/runs/7391864197?check_suite_focus=true

-- regards, DQ

On Mon, Jul 18, 2022, 07:46 Miguel Duarte Barroso @.***> wrote:

@.**** approved this pull request.

Thanks.

Let see what the other maintainers think of this ...

— Reply to this email directly, view it on GitHub https://github.com/k8snetworkplumbingwg/whereabouts/pull/251#pullrequestreview-1041983651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA36GXZJ2LBMNTF3NTDSX6DVUVU4XANCNFSM532NIHCA . You are receiving this because you authored the thread.Message ID: @.***>

deekue avatar Jul 18 '22 17:07 deekue