Skip to content

Add CI check that generated files are up-to-date #246

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

Merged
merged 2 commits into from
Jan 15, 2021

Conversation

amisevsk
Copy link
Collaborator

What does this PR do?

Add a step to the GitHub CI jobs to check that make generate manifests does not change repo (ensure that generated yaml manifests and deepcopy functions are up to date).

Update annotations on devworkspace reconciler to make sure we generate the RBAC we currently are using.

What issues does this PR fix or reference?

Running make manifests changes role.yaml

Is it tested? How?


# Note: fmt is necessary after generate since generated sources will
# fail format check by default.
make generate fmt manifests
Copy link
Member

Choose a reason for hiding this comment

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

it fails with Error patching crds: yq is required

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah bummer. That's probably why we didn't include it it in the first place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added pip install yq and it seems to work without problems

Also add checking manifests to github PR CI

Signed-off-by: Angel Misevski <[email protected]>
Fix issues resulting from Makefile semantics which result in statements
being executed outside of the context of a rule:

* Fix message logging that current-context is not set when there's no
  kubeconfig file (e.g. in CI). This checks if we're in minikube only.
* Fix message warning that operator-sdk is not installed on every
  invocation; only print warning if a rule requires operator-sdk

Signed-off-by: Angel Misevski <[email protected]>
@amisevsk
Copy link
Collaborator Author

amisevsk commented Jan 15, 2021

Fixed CI (:tada:) and also improved Makefile slightly to not print warnings needlessly:

error: current-context is not set
bash: operator-sdk: command not found

were being printed because of how Make interprets lines not part of a rule.

@amisevsk amisevsk force-pushed the check-manifests-in-prcheck branch from 4fb7fa0 to d92aa5e Compare January 15, 2021 19:29
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, sleshchenko

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:
  • OWNERS [JPinkney,amisevsk,sleshchenko]

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

@amisevsk
Copy link
Collaborator Author

I also added a commit (since dropped) that changed a manifest file to verify that CI will fail as we expect.

@amisevsk
Copy link
Collaborator Author

/test v5-devworkspaces-operator-e2e

@amisevsk amisevsk merged commit b434f12 into devfile:master Jan 15, 2021
@amisevsk amisevsk deleted the check-manifests-in-prcheck branch January 15, 2021 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants