-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Makefile,hack/image: build and release targets for helm and ansible binaries #3363
Makefile,hack/image: build and release targets for helm and ansible binaries #3363
Conversation
build/helm-operator-%-s390x-linux-gnu: GOARGS = GOOS=linux GOARCH=s390x | ||
build/helm-operator-%-linux-gnu: GOARGS = GOOS=linux | ||
|
||
build/operator-%: $(SOURCES) ## Build the operator-sdk binary |
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 make this more specific?
build/operator-%: $(SOURCES) ## Build the operator-sdk binary | |
build/operator-sdk$%: $(SOURCES) ## Build the operator-sdk binary |
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 tried that, but it resulted in a weird build error I couldn't figure out how to solve. I'm not a Makefile guru by any means though, so suggestions welcome.
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.
Fixed the above to specify the right target ( That still doesn't work. Let me look into this.$%
).
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.
This is not a blocker, just a cosmetic nit.
build/operator-%: $(SOURCES) ## Build the operator-sdk binary | ||
$(Q)$(GOARGS) go build $(GO_BUILD_ARGS) -o $@ $(SDK_BUILD_PATH) | ||
|
||
build/ansible-%: $(SOURCES) ## Build the ansible-operator binary |
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 make this more specific? Ditto for helm.
build/ansible-%: $(SOURCES) ## Build the ansible-operator binary | |
build/ansible-operator%: $(SOURCES) ## Build the ansible-operator binary |
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.
I am ok with 👍 / Also, I understand that it will be moved to be done in the ansible/helm repo as well when they exist.
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
Description of the change:
Add build targets and rules for the
ansible-operator
andhelm-operator
binariesMotivation for the change:
For 1.0, we want to remove the
operator-sdk run local
command. To continue supporting running helm and ansible operators locally, we will need these binaries to be released so that upcomingmake run
targets for those operator types can download and run the operator binaries locally.Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs