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

Implement the --internal flag for the export kubeconfig command #2217

Closed
gprossliner opened this issue Apr 27, 2021 · 17 comments
Closed

Implement the --internal flag for the export kubeconfig command #2217

gprossliner opened this issue Apr 27, 2021 · 17 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@gprossliner
Copy link
Contributor

What would you like to be added:

The kind get kubeconfig command implements the --internal flag, but kind export kubeconfig doesn't.

Why is this needed:

As outlined and discussed in #2205: To be able to test KubeFed with kind clusters, you need a kubeconfig containing all the clusters. These clusters talk to each other, so the --internal endpoints are needed.

@gprossliner gprossliner added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 27, 2021
@BenTheElder
Copy link
Member

xref: #1558, I wonder if perhaps we should do one flag to select between internal, internal-ip, host (default). Any thoughts? --address-type=internal ?

@BenTheElder
Copy link
Member

--internal is consistent with kind get kubeconfig at least for now.

@gprossliner
Copy link
Contributor Author

I choose --internal to be consistent with kind get kubeconfig (see #2218).

I agree that --address-type=internal|host would be more descriptive, and would be better for my point of view, but I didn't want to introduce a totally new flag, so I just copied the flag from kind get kubeconfig.

@aojea
Copy link
Contributor

aojea commented Apr 28, 2021

my vote for

kind get|export kubeconfig --server-url hostname|internal-ip|host

related to the kubeconfig field we want to get:

- cluster:
    certificate-authority-data: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUM1ekNDQWMrZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFWTVJNd0VRWURWUVFERXdwcmRXSmwKY201bGRHVnpNQjRYRFRJeE1EUXlOekEzTURZeU1Gb1hEVE14TURReU5UQ
TNNRFl5TUZvd0ZURVRNQkVHQTFVRQpBeE1LYTNWaVpYSnVaWFJsY3pDQ0FTSXdEUVlKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTHkrClRCT3l6bVNCeEthWFJwaUQ0Q0xhdmR2NndjRmVSR2JWYkErU0FmZWR2dEZQZHVDcktMMzdlYzJKNWFydXpCSmcKc1l6emlJcm
NKdzVmcFpYT2JFNE9haDVEd0dLeVlMdDZjcEVtNE1QbFF1R1h0QmxPUU5kbit2U0hpT3EzTGxlSQo5Ky84VGtHYU5DREc2VFY2WWpNaGRzYUV2bSs5Mm1pa2k4TjE4dlF5bUpjMzhnL1NXTmYvY1JFNTdqQUkrb1dhCkdOZ0dzVzNWSzdwVGg5aGdlQ2FTbU9RRnVreFZBNXNXVnk4TzV
wVEVRaS9UZllPVHQyUnFwdE9tci9sUGdaeWoKcm45L1JyREJGbkxYMG9SZWNRbUxveGVLU2ZvNG5SYUhURGZvSjRnb0swUXI4d1RCSnRVRjFZMUVYQ3VEdCt6QwpBemdic2pBV1ExR2tGUFhadXFNQ0F3RUFBYU5DTUVBd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4R0ExVWRFd0VCCi93
UUZNQU1CQWY4d0hRWURWUjBPQkJZRUZGaGM2WUZiakhSM2hRNXphWjBqWTJyMUIwckxNQTBHQ1NxR1NJYjMKRFFFQkN3VUFBNElCQVFBd3lRTWUvekdqUHNyeUxPajhkSHVTeFJWT0R5Y3ZEOVd0eUVXQkFyRDNuQVRtRkJTNwpvTHQ1NHNYdkkwelVwTWlRTk5GYW9aSHVZbFM4bXdxe
XlZenFCVElmOEJuNWh3WG5na2txTFpkQktmc0FhMWxHClM3dEtCdnZCSnlXTndXMGxhRUpjME9lZFFzWmtScEFPencycVhURTMrWmMySHU5c2N5bVljdHZIVzViQUoxQUkKK1RHNVFDZ1l3NFVkVmltMFZvYkxlT29jT21XeGpoQ1haY2ZzU2cyaDU3ZDlWN2JEbk44TnVZUG5vQ3hrNG
hoRwo0NmJ4WjMralIwNEtWbVA0TXRuU2FpWklpRHVWNE9nVHlGa1dGN0NKTjY3VURLbU93U2RmNlJEdVpCZzJnTlJpCnN0LzVqMTVYSHZGVGVNQTBVTWcyU3NXLzJkOFBTcFJ0dFF4bwotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==
    server: https://127.0.0.1:39757
  name: kind-kind

@BenTheElder
Copy link
Member

I think server-url sounds like you're supplying the actual server url as the argument, versus address-type

@aojea
Copy link
Contributor

aojea commented Apr 30, 2021

I think server-url sounds like you're supplying the actual server url as the argument, versus address-type

I'm bad at names 🙃 , address-type is good for me,
should we proceed then to the implementation or does this need more discussion?

@gprossliner
Copy link
Contributor Author

What is the current status of the proposal? kind get|export kubeconfig --address-type hostname|internal-ip|host?
So this would be a breaking change for everyone currently doing kind get kubeconfig --internal, right?

What is expected to be returned by the three options:

--address-type: hostname
=> server: https://kind-control-plane:6443

--address-type: internal-ip
=> server: ?

--address-type: host
=> server: https://127.0.0.1:39757

The kind export kubeconfig --internal would be implemened in #2218. I would also change this implementation to a newer spec, if wanted.

@aojea
Copy link
Contributor

aojea commented May 3, 2021

current --internal returns the nodename that should be resolvable from any kind node, and docker container inside the kind network:

docker run --network kind -it busybox sh
/ # ping kind-control-plane
PING kind-control-plane (172.18.0.2): 56 data bytes
64 bytes from 172.18.0.2: seq=0 ttl=64 time=0.117 ms
^C
--- kind-control-plane ping statistics ---

internal-ip can be useful for doing stuff from other network or from the host, if you are using linux,

So this would be a breaking change for everyone currently doing kind get kubeconfig --internal, right?

a new flag can be always added later if there is more demand, but since we currently have users of --internal it makes sense to me to keep using it

@gprossliner
Copy link
Contributor Author

a new flag can be always added later if there is more demand, but since we currently have users of --internal it makes sense to me to keep using it

So you would implement --address-type in addition to --internal for kind export kubeconfig?

@aojea
Copy link
Contributor

aojea commented May 3, 2021

So you would implement --address-type in addition to --internal for kind export kubeconfig?

if there is an use case for that 🤷 , let's wait for Ben, but as you said I think we are already tied by --internal

@gprossliner
Copy link
Contributor Author

In my last message I meant for kind get kubeconfig instead of kind export kubeconfig. Sorry about that.

Personally I don’t get the point of —address-type. Although —internal is not very descriptive, it’s already implemented in kind get kubeconfig. Doing it differently in get and export may be confusing. Having both, —address-type and —internal is even more confusing. Dropping —internal would be a breaking change.

Yes, let’s wait for Ben about this.

@BenTheElder
Copy link
Member

I was wondering if we should perhaps deprecate --internal for the other command and switch for both or if we should just keep --internal. I don't want to hold things up too much on considering that given we already have the other approach, I've just been unavailable intermittently.

Doing it differently in get and export may be confusing. Having both, —address-type and —internal is even more confusing. Dropping —internal would be a breaking change.

It would but we could do it gradually with a clear migration path (don't allow setting both, warn when internal is used, in the future drop internal).

Sorry, I wrote this comment yesterday and missed the "send" click apparently.

@gprossliner
Copy link
Contributor Author

It would but we could do it gradually with a clear migration path (don't allow setting both, warn when internal is used, in the future drop internal).

A clean migration path could be like:

  • Phase 1: Quit with error if one uses --internal and --address-type
  • Phase 1: Output a descriptive warning to stderr if --internal is used
  • Phase 2: Remove --internal from help, and quit with a descriptive error if it is still used
  • Phase 3: Remove --internal completely, if it is still used one will just get an "invalid flag" generic error

I was wondering if we should perhaps deprecate --internal for the other command and switch for both or if we should just keep --internal.

What would we gain on implementing --address-type

  • A more descriptive UX
  • A third option to choose
  • ?

If we want to do this, I would recommend don't use 'internal' at all, and "host" only if it's really the host, not anything in containers.

My suggestion for --address-type would be:

-address-type= URL Host-Resolvable Container-Resolvable
container-name https://kind-control-plane:6443 No Yes
container-ip https://172.18.0.2:6443 Linux only Yes, on 'kind' network
host-port https://127.0.0.1:36995 Yes No

The current default would be "host-port". I would use "container-ip" as the default on Linux, because the URL is resolvable on the host, and internally.

The interesting question is if this is worth the effort. Just implement kind export kubeconfig --internal would be very easy. The only thing it would take is to accept my PR #2218.

Implementing --address-type would be much more work, considering also the migration path. If you want to go this route, I would be happy to assist in that, and rework the PR (or add a new for that).

@aojea
Copy link
Contributor

aojea commented May 5, 2021

I will go with internal right now, can't see an immediate ROI right now with the complexity it requires to move from it as greatly explained in the previous comment

@BenTheElder
Copy link
Member

@aojea see #2246

We should consider the use case for the additional mode, it is not a new request.

@aojea
Copy link
Contributor

aojea commented May 14, 2021

ahh forgot about that sorry

@BenTheElder
Copy link
Member

#2218 this issue is implemented. #2246 remains for the missing part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants