-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
📖 Fix make commands in clusterctl for developers doc #5384
📖 Fix make commands in clusterctl for developers doc #5384
Conversation
@@ -36,15 +36,13 @@ If you want to create a local artifact, follow these instructions: | |||
In order to build artifacts for the CAPI core provider, the kubeadm bootstrap provider and the kubeadm control plane provider: | |||
|
|||
``` | |||
make docker-build REGISTRY=gcr.io/k8s-staging-cluster-api | |||
make generate-manifests REGISTRY=gcr.io/k8s-staging-cluster-api PULL_POLICY=IfNotPresent | |||
make docker-build REGISTRY=gcr.io/k8s-staging-cluster-api PULL_POLICY=IfNotPresent |
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.
I assume the make targets have been refactored at some point. The docker-build
target updates the config folder with the image and the image pull policy. generate-manifests
doesn't change anything (anymore).
@@ -178,8 +176,8 @@ When selecting the `--kubernetes-version`, ensure that the `kindest/node` | |||
image is available. | |||
|
|||
For example, on [docker hub][kind-docker-hub] there is no | |||
image for version `v1.21.2`, therefore creating a CAPD workload cluster with | |||
`--kubernetes-version=v1.21.2` will fail. See [issue 3795] for more details. | |||
image for version `v1.21.3`, therefore creating a CAPD workload cluster with |
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.
Not really important, but v1.21.2 exists now. I suppose when a new v1.21 version is published it will be at least v1.21.5
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.
Let's make the entire sentence more generic. e.g.
For examples, assuming that in in docker hub there is no an image for version vX.Y.Z, then creating a ... will fail
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.
Done
Signed-off-by: Stefan Büringer [email protected]
cc54168
to
e1de82d
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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 |
Signed-off-by: Stefan Büringer [email protected]
What this PR does / why we need it:
Just something I found when testing the latest commit locally.
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 #