Simplify kubernetes context fetching

[noseglid]

I've started notice a small delay when rendering the prompt and tracked
the issue down to the kubernetes part.

Turns out it actually does take a few 100ms to both open the file, and
then run kubectl config view (which also opens the file). This PR gets
both the context and the namespace from the kubectl config view
command.

This change does have the drawback that if there is no namespace at,
the colon : would still be displayed. I'm not familiar enough with
jsonpath to work this out at this time.

[bobthecow]

maybe use string to remove it? piping to …

string trim --right --chars=:

… should do it.

[noseglid]

It does. Thanks!

[bobthecow]

How much of a difference does this make?

I believe (before we added the namespace) the context code was faster than calling kubectl directly. Is there a way to get the namespace without calling kubectl, too?

Should we make the namespace portion optional, and preserve the faster context lookup, if namespace isn't needed? Is it even still faster?

[noseglid]

It does make up some difference. I did some raw measurements, about 50ms (n=10).

Your comment got me thinking a bit and testing some more though. It looks like the current implementation is actually quite broken. the {..namespace} returns all namespaces for every context. So if I have 2 contexts, it returns a string like namespace1 namespace2.

I'll play around with this a bit more and see if I can get it working without invoking kubectl at all.

[noseglid]

The kubernetes config looks like this

{
    "contexts": [
        {
            "name": "context1",
            "context": {
                "cluster": "cluster1",
                "user": "user1",
                "namespace": "namespace1"
            }
        },
        {
            "name": "context2",
            "context": {
                "cluster": "cluster2",
                "user": "user2",
                "namespace": "namespace2"
            }
        }
    ],
    "current-context": "context2"
}

where the current-context entry selects which of the contexts in the contexts array that's currently active. The namespace can then be fetched from there.

I don't see how this can be parsed reliably with the current trivial parsing of the kube config. We'd need to make at least 2 passes (one to get current-context and another to find the context with that name. But even when the context with that name is found, how should we know where to find the namespace? Is it the line above? The line below?

I did look at some other solutions to this, and it looks like using kubectl is the best option. To mitigate the slowness this implementation use cache.

[bobthecow]

Yeah, I agree. It seems unreasonable to parse that without using kubectl. And I don't know that we want to go so far as caching. See https://twitter.com/rob_pike/status/447202124753952768 🙂

The right answer might be to let people opt in/out of including namespaces, and use the current faster implementation when they're only using context?

[noseglid]

I think that sounds reasonable, but arguably a different PR than this one? I think the changes I'm proposing here is sound (reduces complexity a lot, while also making it noticeably faster).

For my use case, these changes are enough. An opt out feature could be implemented once there seems to be a demand for it.

[bobthecow]

i get it sounding like a different pull request, but this pull request is deleting the code we’d want to call for the opt-out-of-namespaces path, which we’d have to add right back in :)

[noseglid]

Sorry for waiting forever to get this done. I agree that it makes sense to make the namespace optional. I've set it default to false (so that it's snappy). As users get more advanced with the k8s feature they might search and find this optional namespace opt-in - then they will notice that sluggishness comes from it and can opt out again if it's not useful to justify the slowness.

[bobthecow]

Thanks!