refactor join command and add preflight check #291
Conversation
|
related issue: #274 |
ycyaoxdu
force-pushed
the
add-preflight
branch
2 times, most recently
from
ae8e9cf to
cba2d75
Compare
|
/assign @qiujian16 |
Signed-off-by: ycyaoxdu <yaoyuchen0626@163.com>
ycyaoxdu
force-pushed
the
add-preflight
branch
from
cba2d75 to
675a4b9
Compare
ycyaoxdu
requested review from
qiujian16
and removed request for
qiujian16 and
yue9944882
ycyaoxdu
changed the title
ycyaoxdu
changed the title
ycyaoxdu
force-pushed
the
add-preflight
branch
2 times, most recently
from
e8b4c39 to
373381f
Compare
ycyaoxdu
changed the title
pkg/helpers/client.go
Outdated
| //it searches in the kube-root-ca.crt configmap. | ||
| // GetCACert returns the CA cert. | ||
| // First by looking in the cluster-info configmap of the kube-public ns and if not found, | ||
| // it searches in the kube-root-ca.crt configmap. |
There was a problem hiding this comment.
it should return empty if kube-root-ca.crt configmap is not found also.
pkg/cmd/join/preflight/checks.go
Outdated
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
| _, err = kubeClient.CertificatesV1().CertificateSigningRequests().List(context.TODO(), metav1.ListOptions{}) |
There was a problem hiding this comment.
maybe just check the version. Or check if it connects to a hub, by discovery?
Signed-off-by: ycyaoxdu <yaoyuchen0626@163.com>
ycyaoxdu
force-pushed
the
add-preflight
branch
from
373381f to
aa0ff26
Compare
pkg/cmd/join/preflight/checks.go
Outdated
| } | ||
| // validate ca | ||
| if c.Config.Clusters[0].Cluster.CertificateAuthorityData == nil { | ||
| return []error{errors.New("no ca detected, creating hub kubeconfig without ca")}, nil |
There was a problem hiding this comment.
this is an warning telling the user that the client has built up with no ca data. I think the user should make the decision whether use --ca-file to specify a ca file himself/herself in this condition @qiujian16
| preflight.HubKubeconfigCheck{ | ||
| Config: o.HubConfig, | ||
| }, | ||
| preflight.KlusterletApiserverCheck{ |
There was a problem hiding this comment.
nit: should we put "check if it is hub" and "check if it is spoke" also as preflight check. Such that this can be used in each cmd? Won't be a blocker for this pr, but worth thinking.
Signed-off-by: ycyaoxdu <yaoyuchen0626@163.com>
pkg/cmd/join/preflight/checks.go
Outdated
| KlusterletApiserver string | ||
| } | ||
|
|
||
| func (c KlusterletApiserverCheck) Check() (warningList []error, errorList []error) { |
There was a problem hiding this comment.
I see what's going on here. But you may want warnings as a []string to avoid confusion.
Signed-off-by: ycyaoxdu <yaoyuchen0626@163.com>
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, ycyaoxdu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Signed-off-by: ycyaoxdu yaoyuchen0626@163.com