Add parameters for node container

[pablochacin]

What this PR does

Add a parameter to create cluster command for specifying
arguments to be passed to the node container run command.

This allows for example, exposing additional ports, or attaching
additional volumes. This is necessary to complement the ability
to create custom node images.

The 'args' parameter receives a string with the parameters
which are appended to the docker run command used for
launching the node container.

For example the command bellow allows mapping port 8000
exposed by a custom node image.

kind create cluster --args "-p 8000:8000

How to test feature

Create cluster exposing port 2379 (etcd)
>kind create cluster --args "--expose 2379 -p 2379:2379

After the cluster has been create, check the port is actually exposed:
> docker inspect kind-1-control-plane | jq '.[].HostConfig.PortBindings[][].HostPort' | grep -o 2379
2379

Signed-off-by: Pablo Chacin [email protected]

[k8s-ci-robot]

Welcome @pablochacin! It looks like this is your first PR to kubernetes-sigs/kind 🎉

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pablochacin
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: bentheelder

If they are not already assigned, you can assign the PR to them by writing /assign @bentheelder in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @pablochacin. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[BenTheElder]

/hold
Thanks for the PR, but I don't think this will make much sense as-is when we merge multi node O(soon) and this is a bit of a compatibility nightmare between versions.
Both additional ports and volumes deserve first class support.

[pablochacin]

@BenTheElder Could you please elaborate on how this PR could make sense. I agree that maybe using more specific arguments like "--port" or "--volume" may better. Do you think this may fit?

Regarding the potential conflict with multi-node, can you please point-me to some specific issues you foresee or the code base for such feature such that I can check it myself?

All that said, I presently needing this feature in order to use Kind as a development environment for the gardener project and I'm planning to add kind as a supported environment.

[BenTheElder]

Sorry for the delay - I focused on getting the initial-multi node in recently as we need it for testing, catching back up on these now 😓

For multi-node and extra flags to docker when creating the nodes you likely want these to be different depending on the node, probably we should add some extra fields to the node configs to allow exposing extra ports and mounting extra volumes.

[BenTheElder]

All that said, I presently needing this feature in order to use Kind as a development environment for the gardener project and I'm planning to add kind as a supported environment.

Also, that sounds awesome :-) Would love to help make that happen.

Can we discuss how extra ports / volumes should work with others in issues first?
It is a high priority that the exposed API / flags stay very stable by the end of the k8s 1.14 release so we can ship something everyone can build on.

I'd like to add something a little less brittle than directly plumbing through flags to docker (which also leaves room for EG #151)

For ports we can use issue #99, and for volumes issue #62 should work I think.

[pablochacin]

Thanks for the feedback @BenTheElder

I was waiting to see how #147 evolved as t has a significant impact on how to expose the node's container engine parameters. I'm now also looking at #151 but apparently, besides changing docker for container.Engine the structure of code, parameters and main logic remains the same as before.

I agree that addressing specific uses cases as you propose in #99 and #62 is a much better approach. Should we consider closing this PR then and move the conversation to those more specific cases?

[BenTheElder]

I think that's the way to go, thanks for understanding 😅

…

On Thu, Jan 17, 2019 at 1:20 AM pablochacin ***@***.***> wrote: Thanks for the feedback @BenTheElder <https://github.com/BenTheElder> I was waiting to see how #147 <#147> evolved as t has a significant impact on how to expose the node's container engine parameters. I'm now also looking at #151 <#151> but apparently, besides changing docker for container.Engine the structure of code, parameters and main logic remains the same as before. I agree that addressing specific uses cases as you propose in #99 <#99> and #62 <#62> is a much better approach. Should we consider closing this PR then and move the conversation to those more specific cases? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#199 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA4Bq6vfFNs1a1EFvJgbVazjgG2B39OJks5vEEBygaJpZM4ZyFMr> .

[BenTheElder]

closing for now, we will definitely aim to prioritize #99 and #62 in this k8s release cycle.