Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃尡 Bump controller-tools to v0.12.1 #2004

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

sbueringer
Copy link
Member

@sbueringer sbueringer commented Jul 13, 2023

What this PR does / why we need it:
Bumps controller-gen to fix the non-deterministic YAML generation.

Thx very much to @ntoofu for fixing the issue in controller-tools (kubernetes-sigs/controller-tools#792)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1757
Fixes #1928

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:


@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 13, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 13, 2023
@sbueringer
Copy link
Member Author

/assign @randomvariable

/cc @chrischdi @killianmuldoon

@sbueringer
Copy link
Member Author

sbueringer commented Jul 13, 2023

I think we should consider cherry-picking this into all branches (if easily doable)

@@ -832,10 +832,10 @@ spec:
being referenced
type: string
required:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are expected:

// TypedLocalObjectReference contains enough information to let you locate the
// typed referenced object inside the same namespace.
// +structType=atomic
type TypedLocalObjectReference struct {
	// APIGroup is the group for the resource being referenced.
	// If APIGroup is not specified, the specified Kind must be in the core API group.
	// For any other third-party types, APIGroup is required.
	// +optional
	APIGroup *string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
	// Kind is the type of resource being referenced
	Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
	// Name is the name of resource being referenced
	Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
}

@chrischdi
Copy link
Member

/lgtm

Did re-run the script successfully with 200 times no diff :-) 馃帀 We're back to deterministic output 馃槃

cd hack/tools
rm bin/controller-gen
make controller-gen
cd ../..
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}

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 13, 2023
@sbueringer
Copy link
Member Author

Thx for verifying and thx for investigating this issue!

@yastij
Copy link
Member

yastij commented Jul 13, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yastij

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make verify-crds intermittently produces different output verify-crds test is flaky
5 participants