make verify-crds intermittently produces different output

[randomvariable]

/kind bug

What steps did you take and what happened:
[A clear and concise description of what the bug is.]

If you run make verify-crds repeatedly, about 20% of the time, the generated CRDs are actually different, usually adding "cluster-api" as a category or setting an atomic map type.

What did you expect to happen:

Consistent CRD generation

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

• Cluster-api-provider-vsphere version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[chrischdi]

I can confirm that this is still the case on main.

[chrischdi]

And also happens at updated controller-gen / controller-tools.

[sbueringer]

We had a similar bug like this in a previous controller-tools release (but that one was fixed). Basically the problem was some non-determinism.

Unfortunately I can't find the old issue/PR anymore. But overall probably requires some deep-dive controller-gen debugging

[sbueringer]

Probably was this one: kubernetes-sigs/controller-tools#680

[sbueringer]

Can you try controller-tools < v0.9.0 if it happens there?

[chrischdi]

With v0.8.0, I only hit the categories added/not added case. The atomic case is gone.

[chrischdi]

Note: I will debug into controller-gen to get down on this.

[chrischdi]

I did take a further look. I can see three cases:

• case A: having categories: cluster-api set / not set on a top-level CRD object
• case B: having x-kubernetes-map-type: atomic set / not set at a property of type object. Often this is some kind of object reference (e.g. bootstrapRef).
• case C: adding/removing that the property apiGroup is required, this also seems to happen on object references, e.g. addressFromPools. At the same time, this may also have the case B.

Some more observations for the Case A:

• if one of the api versions does not define the categories=cluster-api marker, this flakyness happens. Controller-gen seems to be flaky/non-deterministic in this case, maybe because of the random order of reading parsing the files (this is a guess)?
  • Fixable either by taking a deeper look into controller-gen to make the output deterministic
  • Or by adding the categories=cluster-api marker to every api version.

One more observation for all the cases:

• Output seems to be deterministic when only generating for v1beta1.

TODO: inspect more into the other cases, a workaround does not look as simple as for case A (by debugging controller-gen).

[chrischdi]

Simplified reproduction script:

function test:generate() {
  rm -rf tmp tmp2
  mkdir -p tmp tmp2
  eval $CONTROLLER_GEN ${1}
  for i in {1..100}; do
    printf "> %02d\n" "$i";
    eval $CONTROLLER_GEN ${1}2
    diff tmp${2} tmp2${2} || break
  done
}

CONTROLLER_GEN="$(pwd)/hack/tools/bin/controller-gen"
GENARGS="paths=./apis/v1alpha3 \
    paths=./apis/v1alpha4 \
    paths=./apis/v1beta1 \
    crd:crdVersions=v1 \
    output:crd:dir=tmp"

test:generate ${GENARGS}

Note: we have deterministic output when dropping v1alpha3 & v1alpha4 from GENARGS:

[chrischdi]

I did a git bisect on controller-gen/controller-tools.

Looks like this was introduced in kubernetes-sigs/controller-tools#663

There was already a fix after that PR (kubernetes-sigs/controller-tools#687) but this did not fix the issue here.

[ntoofu]

This issue looks the same as #1757.

I sent a PR, but it is not merged yet.

[chrischdi]

Oh wow, that's actually great news 👍 just verified again that the PR would fix the issue 🎉

So yep, definitely seems to be a duplicate of #1757