[tiller-proxy] Improve release list behavior #457
Conversation
cmd/tiller-proxy/main.go
Outdated
| @@ -109,7 +111,7 @@ func main() { | |||
| log.Fatalf("Unable to connect to Tiller: %v", err) | |||
| } | |||
|
|
|||
| proxy = tillerProxy.NewProxy(kubeClient, helmClient) | |||
| proxy = tillerProxy.NewProxy(kubeClient, helmClient, listLimit) | |||
There was a problem hiding this comment.
actually since the Proxy struct is exposed, you can just pass it or removing NewProxy all together.
Actually, do we need to have the proxy.Proxy exposed?
There was a problem hiding this comment.
I am using NewProxy since the properties of Proxy are not exposed so it's not possible to initialize it from a different package:
proxy = &tillerProxy.Proxy{
kubeClient: kubeClient,
}
^ unknown field 'kubeClient' in struct literal of type proxy.Proxy (but does have proxy.kubeClient)
Also I am exposing Proxy to avoid having to pass kubeClient and helmClient for all the methods, is there a benefit if we don't expose it?
pkg/proxy/proxy.go
Outdated
| @@ -81,7 +83,11 @@ type AppOverview struct { | |||
| } | |||
|
|
|||
| func (p *Proxy) get(name, namespace string) (*release.Release, error) { | |||
| list, err := p.helmClient.ListReleases(helm.ReleaseListStatuses(releaseStatuses)) | |||
| list, err := p.helmClient.ListReleases( | |||
| helm.ReleaseListLimit(p.listLimit), | |||
There was a problem hiding this comment.
Do we want the limit here?
Is there any other way to retrieve the single release without relying on a paginated releases list? I am worried that the behavior in which we return not found but actually the page was too small
There was a problem hiding this comment.
I did a quick check in the helm/client.go and there does not seem to be a way, maybe we can use the client.ReleaseStatus but that one seems to only return release.info.
It seems that their CLI will not run into this issue because AFAICT they do the filtering (via the query parameter) after the list them all. So the pagination affects to the results, not to the sample in which they iterate.
There was a problem hiding this comment.
I missed that filter parameter. Using that it will just show the release we are interested in. Thanks!
| @@ -81,7 +82,11 @@ type AppOverview struct { | |||
| } | |||
|
|
|||
| func (p *Proxy) get(name, namespace string) (*release.Release, error) { | |||
| list, err := p.helmClient.ListReleases(helm.ReleaseListStatuses(releaseStatuses)) | |||
| list, err := p.helmClient.ListReleases( | |||
Fixes #435
PD: It's not possible to create a unit test for the patch since the fake helm implementation ignores that limit.