-
Notifications
You must be signed in to change notification settings - Fork 67
Update README to reflect latest code and way for init #127
Conversation
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.
Looking good so far only a couple of comments.
docs/README.md
Outdated
Note you might see following error, wait for a while and retry | ||
``` | ||
Error: "cluster-sample-kubeconfig" not found in namespace "default": secrets "cluster-sample-kubeconfig" not found | ||
``` |
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.
Maybe above this section, we can add a note saying to run kubectl get cluster -w
and wait for the Status
to say Ready
.
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.
Can we just say:
This error means that the status of the cluster is not ready. Please wait until the cluster is ready and try again.
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.
kubectl get cluster -w
the status stuck at
NAME PHASE
cluster-sample Provisioned
for long time..
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.
it should be running per cluster-api project
I guess some pre-requirement not complete so status of cluster is not running ...need find reason later on
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.
Yeah, there was a bug with the published version actually :/ so that object doesn't get updated properly, I fixed it in #136 maybe we can use kubectl get nestedcluster -w
instead since that updates properly and then when that other PR is merged we can switch this back to the cluster resource.
/kind documentation |
42e0cc3
to
3485de7
Compare
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.
Nice job on the updates, if you could just get a couple more pieces in place this will be good to go.
👍
My concern about the #
is a lot of people copy and paste and this is commonly a comment character.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christopherhein, jichenjc 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 |
Thank you so much @jichenjc ! |
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #105