Clarify ordering requirements of client-gen --input #130506
Conversation
|
/assign @BenTheElder |
|
LGTM label has been added. Git tree hash: 3f680aa66f1ad0921129a1d8e1c5846841c12a72
|
|
Hi - how would sorting the input break the pipeline? |
Dep bump to code-generator would cause any build to fail that checks the generated code didn't change. It's fixable by running the code generator when preparing the PR that bumps code-generator dep, but it's arguably a breaking change -- "I didn't change the input of the code generator, why is there a diff in the generated code?" |
|
But there’s never been any compatibility guarantee with these codegen tools and client sets - eg. when context was added to the API surface, gengo => gengov2 etc FWIW I think clientgen doing the sorting will solve more grief than it causes |
Adding context was extremely painful for the community and not something I'd like to repeat.
I hear you. I'm considering it for this case. In the short term I'd like to at least doc the problem. EDIT: Long term we should probably audit all our codegen inputs. There might be other unsorted inputs. |
|
Yeah sounds good I just want to push back that things can’t change in the clientgen code due to pipelines breaking and don’t perform code generation. then we’d never be able to add new methods/features to clients which seems like an odd restriction. |
|
/triage accepted |
I do think this one wouldn't be that painful, you'd have to one-time regenerate on upgrading IF your previous input wasn't sorted. I've been thinking about this and I think it would be better to start sorting it, WIP: #130626 |
I agree. We do need to retain golang backward compatibility of the genereated code ( |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, hashim21223445, jpbetz 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
A "generated code keeps changing order" problem for API gateway was traced down to this. xref: kubernetes-sigs/gateway-api#3652
Which issue(s) this PR fixes:
Ideally, client-gen would sort it's
--inputargument after receiving it to enforce consistent order.Problem is, there are many build pipelines already using client-gen, and changing client-gen to sort after receiving the list would impact their code generation (could be breaking to those pipelines).
So let's at least document that a sorted
--inputis expected.Special notes for your reviewer:
Does this PR introduce a user-facing change?