Better user experience with Kubernetes namespaces for UCP #1034
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,27 @@ | ||
| package kubernetes | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "net/http" | ||
| "net/url" | ||
|
|
||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/cli/command/formatter" | ||
| "github.com/docker/cli/cli/command/stack/options" | ||
| "github.com/pkg/errors" | ||
| core_v1 "k8s.io/api/core/v1" | ||
| apierrs "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // GetStacks lists the kubernetes stacks | ||
| func GetStacks(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) { | ||
| if opts.AllNamespaces || len(opts.Namespaces) == 0 { | ||
| return getStacksWithAllNamespaces(kubeCli, opts) | ||
| } | ||
| return getStacksWithNamespaces(kubeCli, opts, removeDuplicates(opts.Namespaces)) | ||
| } | ||
|
|
||
| func getStacks(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) { | ||
|
|
@@ -44,9 +49,62 @@ func getStacks(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) | |
| return formattedStacks, nil | ||
| } | ||
|
|
||
| func getStacksWithAllNamespaces(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) { | ||
| stacks, err := getStacks(kubeCli, opts) | ||
| if !apierrs.IsForbidden(err) { | ||
| return stacks, err | ||
| } | ||
| namespaces, err2 := getUserVisibleNamespaces(*kubeCli) | ||
| if err2 != nil { | ||
| return nil, errors.Wrap(err2, "failed to query user visible namespaces") | ||
| } | ||
| if namespaces == nil { | ||
| // UCP API not present, fall back to Kubernetes error | ||
| return nil, err | ||
| } | ||
| opts.AllNamespaces = false | ||
| return getStacksWithNamespaces(kubeCli, opts, namespaces) | ||
| } | ||
|
|
||
| func getUserVisibleNamespaces(dockerCli command.Cli) ([]string, error) { | ||
| host := dockerCli.Client().DaemonHost() | ||
| endpoint, err := url.Parse(host) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| endpoint.Scheme = "https" | ||
| endpoint.Path = "/kubernetesNamespaces" | ||
|
Contributor
There was a problem hiding this comment. UCP does not have a versioned API at present. If we're going to rely on UCP endpoints, we need to be careful about making sure that this stays backwards-compatible. Is this UX a requirement? Kubernetes does not have any other operations where you can view a list of only the resources that you have access to.
Member
Author
There was a problem hiding this comment. You're right, this is really fragile and ad hoc, however this is indeed a UX requirement and Kubernetes does not provide this feature…
Collaborator
There was a problem hiding this comment.
@wsong yes, that's the job of UCP to be backwards-compatible on that endpoint (or providing a versionned one)…
Contributor
There was a problem hiding this comment. All right, let's stick with this approach for now, but we'll have to be careful to test this with different combinations of CLI version + UCP version. |
||
| resp, err := dockerCli.Client().HTTPClient().Get(endpoint.String()) | ||
|
Member
There was a problem hiding this comment. Wondering if there are other things that should be set on the request (content-type JSON, custom headers that are configured in the client-configuration, etc); cli/vendor/github.com/docker/docker/client/request.go Lines 225 to 241 in 60930d3
Member
Author
There was a problem hiding this comment. I don't know, however this would not be an issue were the
Member
There was a problem hiding this comment. Well, it's a bit of an odd situation; the
So, should we export the On the "plus" side; when using the I think in an ideal situation, given that UCP defines its own API (which is the Engine remote-API + Additional endpoints/extensions) there would be an official UCP client that we could use.
Collaborator
There was a problem hiding this comment. I do agree with @thaJeztah that it is really an odd situation : we end up hacking the As discussed a bit offline earlier, a cleaner way (way cleaner) would have been to have a kubernetes api extension (same as the current "compose on kubernetes" one) :
Member
Author
There was a problem hiding this comment. Being new to the org I cannot estimate whether this is a realistic solution given the time constraint, @chris-crone any opinion?
Member
There was a problem hiding this comment. @thaJeztah there's an issue against the relevant repo to ensure that this endpoint is properly tested and that its behaviour isn't modified. @vdemeester I agree that those solutions would be cleaner but given the timeline for this; they're not feasible. Since this is a fallback we can replace it or demote it in the fallback list at a later date once we've had time to investigate the cleaner options. |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
| body, err := ioutil.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "received %d status and unable to read response", resp.StatusCode) | ||
| } | ||
| switch resp.StatusCode { | ||
| case http.StatusOK: | ||
| nms := &core_v1.NamespaceList{} | ||
| if err := json.Unmarshal(body, nms); err != nil { | ||
| return nil, errors.Wrapf(err, "unmarshal failed: %s", string(body)) | ||
| } | ||
| namespaces := make([]string, len(nms.Items)) | ||
| for i, namespace := range nms.Items { | ||
| namespaces[i] = namespace.Name | ||
| } | ||
| return namespaces, nil | ||
| case http.StatusNotFound: | ||
| // UCP API not present | ||
| return nil, nil | ||
| default: | ||
| return nil, fmt.Errorf("received %d status while retrieving namespaces: %s", resp.StatusCode, string(body)) | ||
| } | ||
| } | ||
|
|
||
| func getStacksWithNamespaces(kubeCli *KubeCli, opts options.List, namespaces []string) ([]*formatter.Stack, error) { | ||
| stacks := []*formatter.Stack{} | ||
| for _, namespace := range namespaces { | ||
| kubeCli.kubeNamespace = namespace | ||
| ss, err := getStacks(kubeCli, opts) | ||
| if err != nil { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is a bit sad as this is something we already do in the
moby/mobyclientpackage (setting the scheme tohttpswhen tls is configured)..