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

Remove generated files and update docs to generate files as they were #211

Merged
merged 2 commits into from Sep 9, 2022

Conversation

cpmeadors
Copy link
Contributor

@cpmeadors cpmeadors commented Aug 30, 2022

- Description of the problem which is fixed/What is the use case
There are files being tracked that are completely generated by make targets. Fewer files makes maintaining the code easier.

- What I did
Based on the operator-sdk docs [1], I have deleted the generated files from the repo. One file required an additional command line change to generate the file exactly as it was before deleting. That change was added to the DEVELOPMENT.md file. I also add the relevant entries in the .gitignore to keep the files from being added back.

[1] https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/

This fixes: #209 and #208

- How to verify it

If you execute the make file targets all files that were deleted should be recreated exactly the same:

make docker-build
make bundle
make bundle-build CHANNELS=candidate

- Description for the changelog

Generated files have been deleted from the repo

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2022
@openshift-ci
Copy link

openshift-ci bot commented Aug 30, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@cpmeadors cpmeadors changed the title revmove generated files and update docs to generate files as they were Remove generated files and update docs to generate files as they were Aug 31, 2022
@cpmeadors cpmeadors marked this pull request as ready for review August 31, 2022 16:39
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 31, 2022
@openshift-ci openshift-ci bot requested review from bpradipt and jensfr August 31, 2022 16:39
@@ -48,7 +48,7 @@ make docker-push

## Building Operator bundle image
```
make bundle
make bundle CHANNELS=candidate
Copy link
Contributor

Choose a reason for hiding this comment

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

candidate is also what we use in our example subscription in config/samples/deploy, so I think this is a good change

Copy link
Contributor

@jensfr jensfr left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you Cameron!

Copy link
Contributor

@pmores pmores left a comment

Choose a reason for hiding this comment

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

Thanks @cpmeadors !

@pmores
Copy link
Contributor

pmores commented Sep 2, 2022

Merging is blocked though because CI is failing which seems to be due to missing DeepCopyObject() methods in KataConfig, KataConfigList and possibly others. I suspect that our current build sequence is missing something to regenerate zz_generated.deepcopy.go which is removed by this PR.

@pmores
Copy link
Contributor

pmores commented Sep 6, 2022

What's missing seems to be a make generate invocation. Unfortunately I've no idea ATM how CI works but from looking at the Makefile, the generate target is also invoked by test, build and run targets as a prerequisite. So it looks like none of these targets are invoked by CI.

@pmores
Copy link
Contributor

pmores commented Sep 9, 2022

/override

@openshift-ci
Copy link

openshift-ci bot commented Sep 9, 2022

@pmores: /override requires failed status contexts to operate on, but none was given

In response to this:

/override

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pmores
Copy link
Contributor

pmores commented Sep 9, 2022

/override ci/prow/4.9-ci-index-sandboxed-containers-operator-bundle ci/prow/4.9-images ci/prow/4.9-sandboxed-containers-operator-e2e ci/prow/ci-index-sandboxed-containers-operator-bundle ci/prow/images ci/prow/sandboxed-containers-operator-e2e

@openshift-ci
Copy link

openshift-ci bot commented Sep 9, 2022

@pmores: Overrode contexts on behalf of pmores: ci/prow/4.9-ci-index-sandboxed-containers-operator-bundle, ci/prow/4.9-images, ci/prow/4.9-sandboxed-containers-operator-e2e, ci/prow/ci-index-sandboxed-containers-operator-bundle, ci/prow/images, ci/prow/sandboxed-containers-operator-e2e

In response to this:

/override ci/prow/4.9-ci-index-sandboxed-containers-operator-bundle ci/prow/4.9-images ci/prow/4.9-sandboxed-containers-operator-e2e ci/prow/ci-index-sandboxed-containers-operator-bundle ci/prow/images ci/prow/sandboxed-containers-operator-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pmores pmores merged commit 846b467 into openshift:master Sep 9, 2022
@openshift-ci
Copy link

openshift-ci bot commented Sep 9, 2022

@cpmeadors: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@gkurz gkurz mentioned this pull request Sep 20, 2022
pmores added a commit to pmores/sandboxed-containers-operator that referenced this pull request Sep 22, 2022
…image

Since PR openshift#211, generated files are no longer part of this repo.  However,
the build process of the main artifact, controller container image, hasn't
been adapted to the absence of generated files and currently fails.

This change adds generation of generated files to the build process.  It
does so by replacing the bare `go build` command with a `build` Makefile
target invocation.  The 'build' target takes care of both file generation
(as a prerequisite) and building itself.

Several small adjustments are necessary to support running the 'build'
target during the controller image build process.  Apart from Makefile,
also hack/ needs to exist in the builder image as it contains input for
the file generator.  A COPY command had to be adjusted as `make build`
leaves the resulting binary in ./bin instead of ./ .

Finally, 'go mod vendor' is used to avoid an "inconsistent vendoring"
error.  Apparently, 'controller-gen' which runs as part of file generation
invokes 'go' internally without passing it the '-mod=mod' flag which
effectively disables vendoring.  'go mod vendor' is then a way to bring
vendoring into agreement with go.mod.
@cpmeadors cpmeadors deleted the remove-generated-files branch November 10, 2022 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modified files after building
3 participants