kube_codegen: smarter grepping of codegen tags

[marquiz]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Be smarter about finding the input packages for genclient et al. The
previous grep patterns were too generic. This caused code-generator, for
example, to pick up it's own auto-generated packages. In this particular
case having a status field in the type adds a comment to the
autogenerated code like:

// Add a +genclient:noStatus comment above the type...

This, in turn causes problems in some scenarios where the input (api)
and the target package for the auto-generated code reside in separate go
modules

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[marquiz]

/assign @thockin

[ArangoGutierrez]

++

[ArangoGutierrez]

Thanks for this patch Markus, I am dealing with this at kubernetes-sigs/node-feature-discovery#1487

[ArangoGutierrez]

/test pull-kubernetes-node-e2e-containerd

[thockin]

This is OK, but still feels dirty. Maybe we should take the output of the first grep and pipe it to xargs grep -v '^// Code generated by .* DO NOT EDIT.$ (exept not literlly xargs because xargs is dumb WRT maybe-empty input on some OSes)?

Or is that overkill?

[marquiz]

Thanks for the quick review @thockin. I was also thinking something along those lines you suggested but decided to suggest the simplest thing, to avoid breaking anything and not to complicate the shell script wizardry more 😅

If xargs is not an option would adding one more while-read be a possible solution?

        ( kube::codegen::internal::grep -l --null \
            -e '//\s*+genclient' \
            -r "${in_dir}${one_input_api}" \
            --include '*.go' \
            || true \
        ) | while read -r f; do grep -L -e  '^// Code generated by .* DO NOT EDIT.$' $f; done \
        | while read -r -d $'\0' F; do dirname "${F}"; done

[thockin]

We don't need both. Simply matching on \s* seems sufficient. In fact, maybe we should grep for ^\s*//\s*+genclient to really anchor it.

In fact, we should probably do the same in all the other places we use internal::grep, even though they are not a problem (yet). What do you think?

[marquiz]

we should grep for ^\s*//\s*+genclient to really anchor it.

Better 👍

In fact, we should probably do the same in all the other places we use internal::grep, even though they are not a problem (yet). What do you think?

Makes sense to me. I'll make the change and update the PR

[cici37]

/triage accepted

[marquiz]

PR updated, commit message adjusted. @thockin PTAL

/retitle kube_codegen: smarter grepping of codegen tags

[thockin]

/retest

[thockin]

Looks sane to me.

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: d67d744ff82f1f315fa57f6a91e842da82f62bb1

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marquiz, thockin

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:

• staging/src/k8s.io/code-generator/OWNERS [thockin]

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