🐛 (go/v4) fix makefile target to build images ensuring its single responsibility #3353
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/M
| @@ -72,7 +72,7 @@ run: manifests generate fmt vet ## Run a controller from your host. | |||
| # (i.e. docker build --platform linux/arm64 ). However, you must enable docker buildKit for it. | |||
| # More info: https://docs.docker.com/develop/develop-images/build_enhancements/ | |||
| .PHONY: docker-build | |||
| docker-build: test ## Build docker image with the manager. | |||
| docker-build: ## Build docker image with the manager. | |||
There was a problem hiding this comment.
@camilamacedo86 Was there any specific reason to remove the test target apart from it taking long time. I'm still inclined towards us keeping it, as if the test fails the user would know the issue before proceeding to building the image. Imo I don't see any harm in having test target before building image.
There was a problem hiding this comment.
Yes. Following scenarios where does not respect single responsibility one this brings bad side effect:
If you use a tool to create a project and want to either release the project or use it for development purposes, you may encounter a situation where you need to wait for all tests to be complete before building the image. This can be frustrating if you have many tests and only want to build the image.
In addition, when you perform Continuous Integration (CI) to test your project, you may need to run all tests again before building the image if you have an end-to-end (e2e) test. This process can significantly increase the time it takes for your CI job to complete and force you to repeat all tests when you only need to build an image.
Why do I need to run the tests before building the image?
k8s-ci-robot
added
the
needs-rebase
|
@varshaprasad96 thank you for the review, can we merge this one now? |
camilamacedo86
force-pushed
the
fix-makefile-build-image
branch
from
9c21517
to
2796a41
Compare
k8s-ci-robot
removed
the
needs-rebase
k8s-ci-robot
added
the
needs-rebase
camilamacedo86
force-pushed
the
fix-makefile-build-image
branch
from
2796a41
to
4c6b648
Compare
k8s-ci-robot
removed
the
needs-rebase
camilamacedo86
force-pushed
the
fix-makefile-build-image
branch
from
4c6b648
to
b893c11
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, Kavinjsir, varshaprasad96 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 |
|
/retest pull-kubebuilder-e2e-k8s-1-27-1 |
|
@Kavinjsir: The
Use In response to this:
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/test-infra repository. |
|
/retest |
|
/test pull-kubebuilder-e2e-k8s-1-26-0 |
Description
fix makefile target to build images ensuring its single responsibility
Motivation
When we call make build we want only to build the image and run the tests is not a pre-requirement. It can make take too long to build the image.