Add make all-push rule #228
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wongma7 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 |
| sub-image-%: | ||
| $(MAKE) OUTPUT_TYPE=$(call word-hyphen,$*,1) OS=$(call word-hyphen,$*,2) ARCH=$(call word-hyphen,$*,3) OSVERSION=$(call word-hyphen,$*,4) image | ||
|
|
||
| image: .image-$(TAG)-$(OS)-$(ARCH)-$(OSVERSION) |
There was a problem hiding this comment.
It would be nice to document the targets intended to be run by command line like make all-image-docker (I guess you don't expect anyone to call make image TAG=.. OS=.. ARCH=..?). Also, all of these targets should be .PHONY, right? Lastly, might want a check for these variables to be set.
There was a problem hiding this comment.
the main userfacing ones should be make all and make all-push (which is executed by github workflow). But make image has a usecase during CI when we want to only build linux/amd64
The vars are always defaulted by ?=
And i know i can mark them phony but it's annoying to do it for every rule. in particular, the .image should not be phony because it touches a file at the end so that the same image for the same tag+os+arch is not built again
There was a problem hiding this comment.
PHONY'd the rules I could in 2 nd commit xd
|
/lgtm |
Is this a bug fix or adding new feature?
What is this PR about? / Why do we need it? The make all-puish rule makes build procedure consistent with https://github.com/kubernetes-sigs/aws-ebs-csi-driver. I do NOT want to import the complicated low-level manifest manipulation from Makefile of https://github.com/kubernetes-sigs/aws-ebs-csi-driver because it is not necessary for building non-windows images. However the top-level make rules like all-push and variables like REGISTRY/TAG should be consistent.
What testing is done?