GitHub action failed with:

curl -s -f -L https://github.com/krishicks/yaml-patch/releases/download/v0.0.10/yaml_patch_linux -o '_output/tools/bin/yaml-patch'
make: *** [vendor/github.com/openshift/build-machinery-go/make/targets/openshift/../../targets/openshift/yaml-patch.mk:11: ensure-yaml-patch] Error 22

in yaml-patch.mk we get yaml-patch from https://github.com/krishicks/yaml-patch:

curl -s -f -L https://github.com/krishicks/yaml-patch/releases/download/$(YAML_PATCH_VERSION)/yaml_patch_$(GOHOSTOS) -o '$(YAML_PATCH)'

but the repo is not available now:

Any make targets that depend on yq or yaml-patch will fail on a macOS/arm system. yq adds support for macOS/arm in later releases, but yaml-patch is no longer maintained.

We'd like to use make verify in https://github.com/openshift/aws-ebs-csi-driver-operator, however, make verify-codegen fails:

"/bin/bash" "./vendor/k8s.io/code-generator//generate-groups.sh" "all" "github.com/openshift/aws-ebs-csi-driver-operator/pkg/generated" "github.com/openshift/aws-ebs-csi-driver-operator/pkg/apis" "operator:v1alpha1" --output-base ../../.. --go-header-file /dev/null --verify-only
build k8s.io/code-generator/cmd/defaulter-gen: cannot load github.com/spf13/pflag: open /go/src/github.com/openshift/aws-ebs-csi-driver-operator/vendor/k8s.io/code-generator/vendor/github.com/spf13/pflag: no such file or directory
make: *** [verify-codegen] Error 1

Notice double vendor/ in /go/src/github.com/openshift/aws-ebs-csi-driver-operator/vendor/k8s.io/code-generator/vendor/github.com/spf13/pflag.

./vendor/k8s.io/code-generator//generate-groups.sh does cd into ./vendor/k8s.io/code-generator//generate-groups.sh and calls go install from there. And therefore it looks for vendored modules there.

Some (I have not produced an exhaustive list) of the convenience *.mk includes are relying on variables at stages where they don't exist yet. Example repro:

[efried@efried ~]$ cd $(mktemp -d)
[efried@efried tmp.9ZAD5Q9Qzz]$ echo 'include /home/efried/go/src/github.com/openshift/build-machinery-go/make/targets/openshift/controller-gen.mk' > Makefile
[efried@efried tmp.9ZAD5Q9Qzz]$ make ensure-controller-gen 
Installing controller-gen into '_output/tools/bin/controller-gen'
mkdir -p '/bin/'
curl -s -f -L https://github.com/openshift/kubernetes-sigs-controller-tools/releases/download/v0.6.0/controller-gen-linux-amd64 -o '_output/tools/bin/controller-gen'
make: *** [/home/efried/go/src/github.com/openshift/build-machinery-go/make/targets/openshift/controller-gen.mk:11: ensure-controller-gen] Error 23

Here's what's happening:

1. controller-gen.mk is computing controller_gen_dir with := -- which means it happens just once -- during the first pass. Since CONTROLLER_GEN is defined with =, it is computed dynamically at this time, based on PERMANENT_TMP_GOPATH, which is not yet defined because we haven't included tmp.mk yet. So controller_gen_dir gets (permanently) assigned the value /bin/ here.
2. Continuing the first pass, we use the existence of whatever's at $(CONTROLLER_GEN) to determine what the contents of the ensure-controller-gen rule should be. We again dynamically compute CONTROLLER_GEN, but PERMANENT_TMP_GOPATH is still undefined, so CONTROLLER_GEN gets the value /bin/controller-gen. That (presumably) doesn't exist, so we lay down the if version of the rule.
3. Continuing the first pass, we finally include tmp.mk and the PERMANENT_TMP_GOPATH variable is (permanently, because :=) assigned.
4. I run make ensure-controller-gen.
5. The info message computes CONTROLLER_GEN dynamically again, but now since PERMANENT_TMP_GOPATH is defined, it actually looks correct.
6. The mkdir -p "succeeds" (at least on my machine) because /bin already exists (symlink to /usr/bin).
7. The curl command again computes the CONTROLLER_GEN variable properly -- but since the mkdir -p didn't actually create _output/tools/bin, it fails.

I can fix this partway by changing the assignment of controller_gen_dir to be dynamic:

diff --git a/make/targets/openshift/controller-gen.mk b/make/targets/openshift/controller-gen.mk
index 163603c..44efaf3 100644
--- a/make/targets/openshift/controller-gen.mk
+++ b/make/targets/openshift/controller-gen.mk
@@ -2,7 +2,7 @@ self_dir :=$(dir $(lastword $(MAKEFILE_LIST)))
 
 CONTROLLER_GEN_VERSION ?=v0.6.0
 CONTROLLER_GEN ?=$(PERMANENT_TMP_GOPATH)/bin/controller-gen
-controller_gen_dir :=$(dir $(CONTROLLER_GEN))
+controller_gen_dir =$(dir $(CONTROLLER_GEN))
 
 ensure-controller-gen:
 ifeq "" "$(wildcard $(CONTROLLER_GEN))"

Now when I try:

[efried@efried tmp.9ZAD5Q9Qzz]$ make ensure-controller-gen 
Installing controller-gen into '_output/tools/bin/controller-gen'
mkdir -p '_output/tools/bin/'
curl -s -f -L https://github.com/openshift/kubernetes-sigs-controller-tools/releases/download/v0.6.0/controller-gen-linux-amd64 -o '_output/tools/bin/controller-gen'
chmod +x '_output/tools/bin/controller-gen';

Great! Now let's just check that it's idempotent as intended:

[efried@efried tmp.9ZAD5Q9Qzz]$ make ensure-controller-gen 
Installing controller-gen into '_output/tools/bin/controller-gen'
mkdir -p '_output/tools/bin/'
curl -s -f -L https://github.com/openshift/kubernetes-sigs-controller-tools/releases/download/v0.6.0/controller-gen-linux-amd64 -o '_output/tools/bin/controller-gen'
chmod +x '_output/tools/bin/controller-gen';

Nope, it's laying down the file again. Referring back to bullet 2 above, it's because $(wildcard $(CONTROLLER_GEN)) is (still) resolving to /bin/controller-gen, which doesn't exist; so we're still laying down the first version of the rule.

We could fix both problems if we include tmp.mk before PERMANENT_TMP_GOPATH is needed -- i.e. at the top of the file. Based on the comment above that include, the only reason it's at the end is so we can rely on self_dir having the expected value throughout the file up to this point. But:

• Except for generating these includes everywhere, self_dir is only used in four places 1, 2, 3, 4.
• self_dir is patently not part of the "API", since there's no telling what its value might be by the time I try to use it in my root Makefile.

So IMHO it would be cleaner all the way around to do the includes up top and avoid reusing the variable name. In most places, we could just inline the computation, e.g.

diff --git a/make/targets/openshift/controller-gen.mk b/make/targets/openshift/controller-gen.mk
index 163603c..03b962e 100644
--- a/make/targets/openshift/controller-gen.mk
+++ b/make/targets/openshift/controller-gen.mk
@@ -1,4 +1,7 @@
-self_dir :=$(dir $(lastword $(MAKEFILE_LIST)))
+include $(addprefix $(dir $(lastword $(MAKEFILE_LIST))), \
+       ../../lib/golang.mk \
+       ../../lib/tmp.mk \
+)
 
 CONTROLLER_GEN_VERSION ?=v0.6.0
 CONTROLLER_GEN ?=$(PERMANENT_TMP_GOPATH)/bin/controller-gen
@@ -22,10 +25,3 @@ clean-controller-gen:
 
 clean: clean-controller-gen
 
-# We need to be careful to expand all the paths before any include is done
-# or self_dir could be modified for the next include by the included file.
-# Also doing this at the end of the file allows us to use self_dir before it could be modified.
-include $(addprefix $(self_dir), \
-       ../../lib/golang.mk \
-       ../../lib/tmp.mk \
-)

And in those few places where we actually need the variable, we can use one with a unique name so we don't need to worry about conflicts, e.g.

diff --git a/make/targets/openshift/deps.mk b/make/targets/openshift/deps.mk
index 956698c..37a666f 100644
--- a/make/targets/openshift/deps.mk
+++ b/make/targets/openshift/deps.mk
@@ -1,8 +1,8 @@
-self_dir :=$(dir $(lastword $(MAKEFILE_LIST)))
+_self_dir_openshift_deps :=$(dir $(lastword $(MAKEFILE_LIST)))
 
-deps_gomod_mkfile := $(self_dir)/deps-gomod.mk
-deps_glide_mkfile := $(self_dir)/deps-glide.mk
-include $(addprefix $(self_dir), \
+deps_gomod_mkfile := $(_self_dir_openshift_deps)/deps-gomod.mk
+deps_glide_mkfile := $(_self_dir_openshift_deps)/deps-glide.mk
+include $(addprefix $(_self_dir_openshift_deps), \
        ../../lib/golang.mk \
 )
 

Running:

$ docker run -it --rm --workdir=/go/src/github.com/openshift/library-go \
  -v $(pwd):/go/src/github.com/openshift/library-go \
  registry.svc.ci.openshift.org/ocp/builder:golang-1.13 \
  make -C alpha-build-machinery update

on OSX will error out (even when the directory/branch is cleaned with git clean -xfd.

rm -f -r '_output/bin'
if [ -d '_output' ]; then rmdir --ignore-fail-on-non-empty '_output'; fi
[[ ! -d ./_output/ ]] || (ls -l ./_output/ && false)
total 0
drwxr-xr-x 3 root root 96 Sep 24 09:15 tools
make[1]: *** [test-cross-build] Error 1

full gist: https://gist.github.com/mfojtik/5e55f8b7c75d8d880a9851dc5241ae0b

The following branches are being fast-forwarded from the current development branch (master) as placeholders for future releases. No merging is allowed into these release branches until they are unfrozen for production release.

• release-4.14
• release-4.15

For more information, see the branching documentation.

The following branches are being fast-forwarded from the current development branch (master) as placeholders for future releases. No merging is allowed into these release branches until they are unfrozen for production release.

• release-4.10
• release-4.9

Contact the Test Platform or Automated Release teams for more information.

Not entirely sure if this is the right place to report this. When I tried using this to generate schemas for the machine-config-operator, and it generated a snippet as follows:

           maxUnavailable:
              description: maxUnavailable specifies the percentage or constant number
                of machines that can be updating at any given time. default is 1.
              anyOf:
              - type: string
              - type: integer

Which resulted in an error when attempting to apply:

* spec.validation.openAPIV3Schema.properties[spec].properties[maxUnavailable].anyOf[0].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[maxUnavailable].anyOf[1].type: Forbidden: must be empty to be structural
* spec.validation.openAPIV3Schema.properties[spec].properties[maxUnavailable].type: Required value: must not be empty for specified object fields

From https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#intorstring it should be

           maxUnavailable:
              description: maxUnavailable specifies the percentage or constant number
                of machines that can be updating at any given time. default is 1.
              anyOf:
              - type: integer
              - type: string
              x-kubernetes-int-or-string: true

instead.

Looking upstream this seems to have been fixed in kubernetes-sigs/controller-tools#360 ?

In openshift/service-ca-operator :

% make verify
…
sed -i '/private/var/folders/…/T/tmp.HB7IlAjH/05_deploy-ibm-cloud-managed.yaml' -e '1s/^/# *** AUTOMATICALLY GENERATED FILE - DO NOT EDIT ***\n/'
sed: -i may not be used with stdin

Apparently the suffix to option to -i is mandatory, so there’s no way to express exactly the above in a way that works on both Linux (--in-place= is not recognized by macOS) and macOS (-i '' on Linux treats '' as a file name.)

The following branches are being fast-forwarded from the current development branch (master) as placeholders for future releases. No merging is allowed into these release branches until they are unfrozen for production release.

• release-4.12

For more information, see the branching documentation.

RIght now everything is in the main package so downstream consumers can't import the validation regex or the config struct.

make verify-deps does a mktemp -d to create a work directory, but doesn't clean it up.

Since /tmp is usually of type tmpfs, the directories sit around until reboot in ram.

So, the temporary directory should be cleaned up, including if the recipe fails.

The new configuration options work great for PRs that are the re-base, since the new logic is to compute the merge-base between the downstream and the upstream, and only verify past that. However, for every PR after the first one, there will be merge commits in there which won't follow the UPSTREAM: pattern. Likely we just want to ignore merge commits during validation?