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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

controller-gen use of line breaks varies minimally on operator system #832

Closed
gabemontero opened this issue Jul 12, 2021 · 11 comments
Closed
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/dependency-change Categorizes issue or PR as related to changing dependencies kind/documentation Categorizes issue or PR as related to documentation.
Milestone

Comments

@gabemontero
Copy link
Member

So make generate-crds behaves differently with respect to line breaks

It works as desired on ubuntu, but not

  • fedora
  • macOS

There is an upstream issue kubernetes-sigs/controller-tools#514 which has been closed but folks are still reporting seeing problems, including @imjasonh from our community

Currently, our github action verify job runs make generate-crds to verify consistency, and since that runs on ubuntu, you need to conform to the ubuntu behavior.

I've started working on a tailored ubuntu based image where if you mount your local branch into a subsequent container running that image, you can run make generate-crds from within the container but update your local branch properly.

We could either publish that image to a public shipwright communit repo on quay.io, and then add a make target to leverage that via a container with local mount, or even add a target that builds that image locally, and then runs a container with local mount. My only hiccup there is the install of golang on ubuntu require more interaction besides what the "yes to install prompts" option covers, as it wants you to select geography, etc. Figuring out how to pre-supply those choices so interaction is not involved would be required.

And ultimately, some updating of the contributor guide and maybe PR template to warn contributors of this seems warranted.

@gabemontero gabemontero added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. kind/dependency-change Categorizes issue or PR as related to changing dependencies labels Jul 12, 2021
@sbose78
Copy link
Member

sbose78 commented Jul 14, 2021

Groomed.

@MayukhSobo
Copy link
Contributor

@gabemontero planning to work on #586 which would include this as well

@gabemontero
Copy link
Member Author

@gabemontero planning to work on #586 which would include this as well

awesome thanks @MayukhSobo

@adambkaplan
Copy link
Member

Accepting this into the general backlog.

@akram
Copy link
Contributor

akram commented Sep 22, 2021

#873 should also helps working around this issue until kustomize and controller-gen bump their version of kyaml

The workaround while using the development tools image is to run:

CONTAINER_ENGINE=podman make container-make TARGET=generate-crds

@HeavyWombat
Copy link
Contributor

@gabemontero @adambkaplan I think the issue was resolved in the meantime. On my local machine, I deleted the CRDs and ran make generate-crds in different containers to verify that the CRDs are regenerated the same way they were before. For me, it worked on my local macOS, Ubuntu, and RHEL (UBI).

@gabemontero
Copy link
Member Author

thanks for the info @HeavyWombat , that said
In RH dev we actually run with "fedora" here instead of rhel (part of RH dev validating the upstream before it gets downstreamed to rhel).

It is on fedora that we are hitting issues.

We are exploring here locally some between @akram and myself when we have cycles

But given that, I want to keep this open until we reach a conclusion there.

@akram
Copy link
Contributor

akram commented Sep 23, 2021

@HeavyWombat I tried yesterday on macosx with controller-gen@0.5.0 and the issue still happens. I have no line wrap on macos, but on ubuntu there is line wrap at 80 chars.

@akram
Copy link
Contributor

akram commented Sep 24, 2021

/assign akram

@SaschaSchwarze0
Copy link
Member

The update to the v0.6.2 version of controller-gen imo harminized this (= always uses line breaks). @gabemontero can you confirm that you can run this with the right version of controller-gen these days also on your machine without causing changes to line breaks?

@gabemontero
Copy link
Member Author

Confirmed!!

Added a new Foo string field to the BuildSpec and make generate only made the additions for my new field, and did not reformat existing fields.

Good catch - thanks @SaschaSchwarze0

closing this out

good-first-issues automation moved this from Build Controller to Done Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/dependency-change Categorizes issue or PR as related to changing dependencies kind/documentation Categorizes issue or PR as related to documentation.
Projects
Development

Successfully merging a pull request may close this issue.

7 participants