✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' #1817

wking · 2020-11-11T22:00:22Z

Because folks may have docker aliased to podman, and Podman prefers options before positional arguments (containers/podman#2811):

$ podman build --help | grep CONTEXT-DIRECTORY
   podman build [command options] CONTEXT-DIRECTORY | URL

Generated with:

$ sed -i 's/ . -t \([^ ]*\)/build -t \1 ./' $(git grep -l 'docker.*build.* \. ')

k8s-ci-robot · 2020-11-11T22:00:30Z

Welcome @wking!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-11-11T22:00:30Z

Hi @wking. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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/test-infra repository.

docs/kubebuilder_v0_v1_difference.md

Adirio · 2020-11-12T06:32:07Z

/ok-to-test

Adirio · 2020-11-12T06:34:27Z

Add 🌱 at the start of the title of the PR please

camilamacedo86

Just a few nits:

Revert the change in the doc docs/kubebuilder_v0_v1_difference.md since this change should not affect that.
Ensure that the commits are squashed
Update the title of this PR to ad the mojo (see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/CONTRIBUTING.md#pr-process).

Because folks may have 'docker' aliased to 'podman', and Podman prefers options before positional arguments [1]: $ podman build --help | grep CONTEXT-DIRECTORY podman build [command options] CONTEXT-DIRECTORY | URL Generated with: $ sed -i 's/ . -t $[^ ]*$/ -t \1 ./' $(git grep -l 'docker.*build.* \. ') $ git checkout HEAD -- docs/kubebuilder_v0_v1_difference.md The docs checkout leaves historical docs alone [2]. [1]: containers/podman#2811 [2]: kubernetes-sigs#1817 (comment)

wking · 2020-11-16T18:09:58Z

Just a few nits...

I think I got all of these with 7769cd9 -> 3b13378.

camilamacedo86 · 2020-11-16T18:11:07Z

Hi @wking,

IMO it is not a bug. So, shows that the mojo should be ✨. Could you please update that?

@Adirio , the 🌱 is for Infra/Tests/Other. However, the proposed change here changes the Makefile of the built projects to allow users to easily work with the postman. So, it shows not the most appropriate one.

Because folks may have 'docker' aliased to 'podman', and Podman prefers options before positional arguments [1]: $ podman build --help | grep CONTEXT-DIRECTORY podman build [command options] CONTEXT-DIRECTORY | URL Generated with: $ sed -i 's/ . -t $[^ ]*$/ -t \1 ./' $(git grep -l 'docker.*build.* \. ') $ git checkout HEAD -- docs/kubebuilder_v0_v1_difference.md The docs checkout leaves historical docs alone [2]. [1]: containers/podman#2811 [2]: kubernetes-sigs#1817 (comment)

wking · 2020-11-16T18:47:13Z

IMO it is not a bug. So, shows that the mojo should be ✨. Could you please update that?

This moves kubebuilder Makefile rules from "silently build images without tagging them" to "correctly tag built images" on older podman, and from "error out of attempted builds" to "correctly build and tag images" for newer podman. That seems like a bug to me, although I agree that the lack of a Docker-CLI specification distinct from the docker implementation makes it unclear whether fixing broken podman support is a kubebuilder bug or feature. But moved to ✨ with 3b13378 -> 1ef003d.

Adirio · 2020-11-16T20:06:33Z

/test pull-kubebuilder-e2e-k8s-1-14-1

camilamacedo86 · 2020-11-16T20:09:30Z

docs/book/src/multiversion-tutorial/testdata/project/Makefile

@@ -63,7 +63,7 @@ generate: controller-gen

 # Build the docker image
 docker-build: test
-	docker build . -t ${IMG}
+	docker build -t ${IMG} .


It is not a breaking change. However, just to do a quick poll.
Do you think that it should be addressed to v2+ or only v3-alpha plugin @estroz @Adirio @gabbifish ?

I would say it is a breaking change (v2 projects scaffolded before this PR will have a different Makefile), and thsu, only to v3-alpha

IMO: v2 projects scaffolded before this PR will have a different Makefile does not characterize a breaking change since it will not break the projects scaffolds before this change. However, we have been very restrictive regards changes in v2 and it is not a critical bug fix for the tool which might is a good argumentation to be only v3+.

+1.

Hi @wking,

Could you please revert the changes in pkg/plugin/v2 and run make generate and push it again. I think would be better to address it only for v3-alpha as well. Could you also add (go/v3-alpha) on the title to make it clear enough? Then, I think would be fine to get this one merged.

...v2 projects scaffolded before this PR will have a different Makefile...

Ideally there would be a flow to easily re-scaffold projects to pull in minor stuff like this, instead of delaying all of that and batching it in major-version bumps. Is there a ticket around that I can subscribe to? If not, should I file one?

Could you please revert the changes in pkg/plugin/v2 and...

I think I've dropped the v2 stuff with 1ef003d -> cced240.

The philosophy we have had until now is that once scaffolded, kubebuilder doesn't own the file, user can modify them (actually, they will modify files, kubebuilder provides the scheleton but you need to code parts) so once scaffolded we are not allowed to delete a file, and thus, we can't re-scaffold it later. The only exception is that some code is injected into some files with some special markers.

The philosophy we have had until now is that once scaffolded, kubebuilder doesn't own the file, user can modify them...

That offloads a lot of maintenance cost to folks maintaining the consuming project, doesn't it? E.g. if we landed a v2 fix here, every kubebuilder-scaffolded project would still be on their own to discover, diagnose, and manually patch their own project to pick up the fix for podman compatibility. It seems like there should be a way to maintain kubebuilder input for portions of the scaffolding, and make it easy for downstream maintainers to ask "do the kubebuilder maintainers have any improvements to suggest for my project?", see a set of suggestions, and then accept or reject them as they see fit. For one approach to this (tracking the scaffolding in a separate branch which is periodically merged into downstream code), see here. I expect there are many alternative approaches which would also allow for downstream maintainers to delegate scaffolding maintenance to the kubebuilder maintainers.

Because folks may have 'docker' aliased to 'podman', and Podman prefers options before positional arguments [1]: $ podman build --help | grep CONTEXT-DIRECTORY podman build [command options] CONTEXT-DIRECTORY | URL Generated with: $ sed -i 's/ . -t $[^ ]*$/ -t \1 ./' $(git grep -l 'docker.*build.* \. ') $ git checkout HEAD -- docs/kubebuilder_v0_v1_difference.md $ git checkout HEAD -- pkg/plugin/v2 testdata/project-v2* The docs checkout leaves historical docs alone [2]. The v2 checkouts are because Adrián and Camila consider this minor and not worth fixing in v2 [3]. [1]: containers/podman#2811 [2]: kubernetes-sigs#1817 (comment) [3]: kubernetes-sigs#1817 (comment)

docs/book/src/cronjob-tutorial/testdata/project/Makefile

docs/book/src/multiversion-tutorial/testdata/project/Makefile

Because folks may have 'docker' aliased to 'podman', and Podman prefers options before positional arguments [1]: $ podman build --help | grep CONTEXT-DIRECTORY podman build [command options] CONTEXT-DIRECTORY | URL Generated with: $ sed -i 's/ . -t $[^ ]*$/ -t \1 ./' $(git grep -l 'docker.*build.* \. ') $ git checkout HEAD -- docs $ git checkout HEAD -- pkg/plugin/v2 testdata/project-v2* The docs checkout leaves historical docs alone [2,3,4]. The v2 checkouts are because Adrián and Camila consider this minor and not worth fixing in v2 [5]. [1]: containers/podman#2811 [2]: kubernetes-sigs#1817 (comment) [3]: kubernetes-sigs#1817 (comment) [4]: kubernetes-sigs#1817 (comment) [5]: kubernetes-sigs#1817 (comment)

Adirio · 2020-11-16T22:45:04Z

/test pull-kubebuilder-test

Adirio

/lgtm

estroz · 2020-11-16T22:50:43Z

/approve

k8s-ci-robot · 2020-11-16T22:50:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, estroz, wking

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:

~~OWNERS~~ [estroz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 11, 2020

k8s-ci-robot requested review from estroz and joelanford November 11, 2020 22:00

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 11, 2020

wking mentioned this pull request Nov 11, 2020

*: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' operator-framework/operator-sdk#4226

Closed

wking force-pushed the podman-build-compat branch from 9e997ee to 7769cd9 Compare November 11, 2020 22:01

estroz reviewed Nov 12, 2020

View reviewed changes

docs/kubebuilder_v0_v1_difference.md Outdated Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 12, 2020

camilamacedo86 reviewed Nov 16, 2020

View reviewed changes

wking force-pushed the podman-build-compat branch from 7769cd9 to 3b13378 Compare November 16, 2020 18:08

wking changed the title *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' 🐛 *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' Nov 16, 2020

wking changed the title ~~🐛 *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .'~~ ✨ *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' Nov 16, 2020

wking force-pushed the podman-build-compat branch from 3b13378 to 1ef003d Compare November 16, 2020 18:44

camilamacedo86 reviewed Nov 16, 2020

View reviewed changes

wking force-pushed the podman-build-compat branch from 1ef003d to cced240 Compare November 16, 2020 21:25

wking changed the title ~~✨ *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .'~~ ✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' Nov 16, 2020

Adirio suggested changes Nov 16, 2020

View reviewed changes

docs/book/src/cronjob-tutorial/testdata/project/Makefile Outdated Show resolved Hide resolved

docs/book/src/multiversion-tutorial/testdata/project/Makefile Outdated Show resolved Hide resolved

wking force-pushed the podman-build-compat branch from cced240 to ccb1127 Compare November 16, 2020 22:06

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 16, 2020

Adirio approved these changes Nov 16, 2020

View reviewed changes

k8s-ci-robot assigned Adirio Nov 16, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2020

k8s-ci-robot merged commit c4c8200 into kubernetes-sigs:master Nov 16, 2020

wking deleted the podman-build-compat branch November 17, 2020 01:02

camilamacedo86 added this to the v3.0.0 milestone Nov 18, 2020

camilamacedo86 mentioned this pull request Feb 18, 2021

New Release - proposing 3.0.0 stable #2029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' #1817

✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' #1817

wking commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

Adirio commented Nov 12, 2020

Adirio commented Nov 12, 2020

camilamacedo86 left a comment •

edited

Loading

wking commented Nov 16, 2020

camilamacedo86 commented Nov 16, 2020 •

edited

Loading

wking commented Nov 16, 2020

Adirio commented Nov 16, 2020

camilamacedo86 Nov 16, 2020

Adirio Nov 16, 2020

camilamacedo86 Nov 16, 2020

camilamacedo86 Nov 16, 2020

wking Nov 16, 2020

Adirio Nov 16, 2020

wking Nov 16, 2020 •

edited

Loading

Adirio commented Nov 16, 2020

Adirio left a comment

estroz commented Nov 16, 2020

k8s-ci-robot commented Nov 16, 2020

✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' #1817

✨ (go/v3-alpha) *: Replace 'docker build . -t ${IMG}' with '-t ${IMG} .' #1817

Conversation

wking commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

k8s-ci-robot commented Nov 11, 2020

Adirio commented Nov 12, 2020

Adirio commented Nov 12, 2020

camilamacedo86 left a comment • edited Loading

Choose a reason for hiding this comment

wking commented Nov 16, 2020

camilamacedo86 commented Nov 16, 2020 • edited Loading

wking commented Nov 16, 2020

Adirio commented Nov 16, 2020

camilamacedo86 Nov 16, 2020

Choose a reason for hiding this comment

Adirio Nov 16, 2020

Choose a reason for hiding this comment

camilamacedo86 Nov 16, 2020

Choose a reason for hiding this comment

camilamacedo86 Nov 16, 2020

Choose a reason for hiding this comment

wking Nov 16, 2020

Choose a reason for hiding this comment

Adirio Nov 16, 2020

Choose a reason for hiding this comment

wking Nov 16, 2020 • edited Loading

Choose a reason for hiding this comment

Adirio commented Nov 16, 2020

Adirio left a comment

Choose a reason for hiding this comment

estroz commented Nov 16, 2020

k8s-ci-robot commented Nov 16, 2020

camilamacedo86 left a comment •

edited

Loading

camilamacedo86 commented Nov 16, 2020 •

edited

Loading

wking Nov 16, 2020 •

edited

Loading