bug: Handle empty CRD directories in Makefile install/uninstall targets #5042
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @nerdeveloper. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nerdeveloper The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/L
|
Coincidentally, I came across this today as well. I did the following: .PHONY: install
install: manifests kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config.
@[ -d "$(CRD_DIR)" ] && $(KUSTOMIZE) build "$(CRD_DIR)" | $(KUBECTL) apply -f - || { \
echo "No CRDs found in $(CRD_DIR) (skipping install)"; \
}
.PHONY: uninstall
uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion.
@[ -d "$(CRD_DIR)" ] && $(KUSTOMIZE) build "$(CRD_DIR)" | $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f - || { \
echo "No CRDs found in $(CRD_DIR) (skipping uninstall)"; \
} |
| *.log | ||
|
|
||
| # If you use vendor, remove this ignore or whitelist it instead | ||
| vendor/ |
There was a problem hiding this comment.
We have some problems here. For example:
- If we add all possible paths to be ignored it will be hard to maintain
- if we try to be generic like: WIP: 🐛 (go/v4): Fix Docker builds failing when projects don’t include
apis/,controllers/, orwebhooksby updating.dockerignoreto allow only Go source files and module metadata. #5045 ( it seems that is not working )
So, we need to think in a solution that will be valid for all scenarios
Will not add files that should not be added
and is easier to keep maintained as generic enough
There was a problem hiding this comment.
One option might keep the default scaffold with:
# Copy the go source
COPY cmd/main.go cmd/main.go
AND just add
COPY api/ api/
when we create an API
AS
COPY internal/ internal/
When we create an webhook or controller
We will need to analyse all options properly and get the best approach
We must be very very careful with ANY change that impact end users in the default scaffolds.
nerdeveloper
force-pushed
the
fix/make-install-no-crd
branch
from
07ff611 to
8e61ac2
Compare
| @@ -120,4 +120,44 @@ jobs: | |||
| - name: Testing make test-e2e for project-v4-multigroup | |||
There was a problem hiding this comment.
Could you please rebase this PR on the master branch?
And then ensure that we have only 1 commit for the required changes?
nerdeveloper
force-pushed
the
fix/make-install-no-crd
branch
from
cf3a031 to
9d043ce
Compare
| !go.mod | ||
| !go.sum | ||
| # Ignore build and test binaries. | ||
| bin/ |
There was a problem hiding this comment.
This file should not be changed
Can you please revert and ensure that the PR has only the required changes?
|
|
||
| # Build | ||
| # the GOARCH has no default value to allow the binary to be built according to the host where the command | ||
| # the GOARCH has not a default value to allow the binary be built according to the host where the command | ||
| # was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO | ||
| # the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore, | ||
| # by leaving it empty we can ensure that the container and binary shipped on it will have the same platform. |
There was a problem hiding this comment.
This file should not be changed
Can you please revert and ensure that the PR has only the required changes?
nerdeveloper
force-pushed
the
fix/make-install-no-crd
branch
2 times, most recently
from
f3edcf5 to
1aa7af6
Compare
- Gracefully no-op install/uninstall when no CRDs exist - Update sample Makefiles accordingly and add CI job to validate empty project - Revert Dockerfile and .dockerignore changes; keep templates and samples matching upstream
nerdeveloper
force-pushed
the
fix/make-install-no-crd
branch
from
1aa7af6 to
03fddac
Compare
k8s-ci-robot
added
size/M
Description
This PR fixes an issue where
make installfails on freshly initialized Kubebuilder projects that have no APIs created yet. Thescaffolded Makefile now gracefully handles missing CRD directories.
Motivation
When users run
kubebuilder initwithout creating any APIs, theconfig/crddirectory doesn't exist. This causesmake installto fail with: Error: must build at directory: not a valid directory: evalsymlink failure on 'config/crd' : lstat .../config/crd: no such file or directoryThis breaks the basic e2e tests and GitHub Actions workflows for projects in their initial state.
Changes
Updated Makefile template (
pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go):installtarget to check if CRDs exist before attempting to installuninstalltarget with the same graceful handlingAdded e2e test validation (
.github/workflows/test-e2e-samples.yml):e2e-test-basic-projectto test Scenario 1 from Test GitHub Actions scaffolded and fix them if required #4977Testing
make installnow outputs "No CRDs to install; skipping" instead of failingmake uninstallworks similarlyFixes #5033
Addresses Scenario 1 from #4977