Conversation
|
A bit more context, this branch fell out of work on #2337, where we need to introduce multi-stage support into |
|
Integration test results for 8dcbf0a: fail 😕 |
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
⭐️ Awesome! This is such a great cleanup.
| err = json.Unmarshal(bytes, &versionInfo) | ||
| return &versionInfo, err | ||
| func (kubeAPI *KubernetesAPI) GetVersionInfo() (*version.Info, error) { | ||
| return kubeAPI.Discovery().ServerVersion() |
There was a problem hiding this comment.
Oh man the changes in this file are so 😻
pkg/k8s/api.go
Outdated
| // configured cluster | ||
| func NewAPI(configPath, kubeContext string) (*KubernetesAPI, error) { | ||
| // configured cluster. | ||
| func NewAPI(configPath, kubeContext string, timeout time.Duration) (*KubernetesAPI, error) { |
There was a problem hiding this comment.
Not your change, but can I bug you to move this func to the top of this file, right below where the KubernetesAPI struct is defined? It feels weird to have the initializer func defined so far down in the file.
Numerous codepaths have emerged that create k8s configs, k8s clients, and make k8s api requests. This branch consolidates k8s client creation and APIs. The primary change migrates most codepaths to call `k8s.NewAPI` to instantiate a `KubernetesAPI` struct from `pkg`. `KubernetesAPI` implements the `kubernetes.Interface` (clientset) interface, and also persists a `client-go` `rest.Config`. Specific list of changes: - removes manual GET requests from `k8s.KubernetesAPI`, in favor of clientsets - replaces most calls to `k8s.GetConfig`+`kubernetes.NewForConfig` with a single `k8s.NewAPI` - introduces a `timeout` param to `k8s.NewAPI`, currently only used by healthchecks - removes `NewClientSet` in `controller/k8s/clientset.go` in favor of `k8s.NewAPI` - removes `httpClient` and `clientset` from `HealthChecker`, use `KubernetesAPI` instead Signed-off-by: Andrew Seigner <[email protected]>
Signed-off-by: Andrew Seigner <[email protected]>
siggy
force-pushed
the
siggy/check-k8s-smpfy
branch
from
8dcbf0a to
3c95271
Compare
|
Integration test results for 3c95271: fail 😕 |
The `linkerd check --proxy` command checks for proxies in all namespaces, if the `--namespace` flag is not set. PR #2747 modified the behavior of `KubernetesAPI.NamespaceExists`. Previously it would succeed if given an emptry string for a namespace. Now it fails with a `resource name may not be empty` error (for k8s server `v1.10.11`), or a not found error (for our fake test client). Modify the data plane proxy namespace check to return success if the namespace is not set. Signed-off-by: Andrew Seigner <[email protected]> fix lint Signed-off-by: Andrew Seigner <[email protected]>
The `linkerd check --proxy` command checks for proxies in all namespaces, if the `--namespace` flag is not set. PR #2747 modified the behavior of `KubernetesAPI.NamespaceExists`. Previously it would succeed if given an emptry string for a namespace. Now it fails with a `resource name may not be empty` error (for k8s server `v1.10.11`), or a not found error (for our fake test client). Modify the data plane proxy namespace check to return success if the namespace is not set. Signed-off-by: Andrew Seigner <[email protected]>
3 participants
Numerous codepaths have emerged that create k8s configs, k8s clients,
and make k8s api requests.
This branch consolidates k8s client creation and APIs. The primary
change migrates most codepaths to call
k8s.NewAPIto instantiate aKubernetesAPIstruct frompkg.KubernetesAPIimplements thekubernetes.Interface(clientset) interface, and also persists aclient-gorest.Config.Specific list of changes:
k8s.KubernetesAPI, in favor ofclientsets
k8s.GetConfig+kubernetes.NewForConfigwitha single
k8s.NewAPItimeoutparam tok8s.NewAPI, currently only used byhealthchecks
NewClientSetincontroller/k8s/clientset.goin favor ofk8s.NewAPIhttpClientandclientsetfromHealthChecker, useKubernetesAPIinsteadSigned-off-by: Andrew Seigner [email protected]