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.

Sign up for GitHub

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

Jump to bottom

Ensure contrib/scripts/lock-check.sh runs as a precheck and as a GitHub Action #11730

Closed
1 of 2 tasks
christarazi opened this issue May 27, 2020 · 4 comments · Fixed by #12693
Closed
1 of 2 tasks

Ensure contrib/scripts/lock-check.sh runs as a precheck and as a GitHub Action #11730

christarazi opened this issue May 27, 2020 · 4 comments · Fixed by #12693
Assignees
@tklauser
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ci/flake This is a known failure that occurs in the tree. Please investigate me! good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium.

Comments

@christarazi
Copy link
Member

christarazi commented May 27, 2020 •
edited by tklauser

This would eliminate PRs like this in the future, and provide quicker feedback to the offending PR. #11729

Items:

  • Create Makefile target to run this precheck
  • Create a GH Action that runs the above
@christarazi christarazi added good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ci/flake This is a known failure that occurs in the tree. Please investigate me! labels May 27, 2020
@joestringer
Copy link
Member

For what it's worth, this should be part of a standard make execution, which:

  1. All developers should run prior to submission; and
  2. We should have a GH action that runs this target.

@tklauser
Copy link
Member

FWIW, the script is already invoked as part of the postcheck target:

cilium/Makefile

Lines 507 to 512 in e871f28

postcheck: build
$(QUIET)$(MAKE) $(SUBMAKEOPTS) -C Documentation update-cmdref check
@$(ECHO_CHECK) contrib/scripts/lock-check.sh
$(QUIET) contrib/scripts/lock-check.sh
@$(ECHO_CHECK) contrib/scripts/rand-check.sh
$(QUIET) contrib/scripts/rand-check.sh

which in turn is part of the all target (which is the default target if you run make without a specific target):

cilium/Makefile

Line 130 in e871f28

all: precheck build postcheck

which leaves me wondering why it only failed in the CI runtime test, but not Travis CI.

@pchaigno
Copy link
Member

which leaves me wondering why it only failed in the CI runtime test, but not Travis CI.

Travis CI doesn't do a full make. It only compiles what it needs to run the unprivileged unit tests AFAIK.

@christarazi christarazi changed the title Move contrib/scripts/lock-check.sh to be a GitHub Action instead of being run during the Runtime tests Ensure contrib/scripts/lock-check.sh runs as a precheck and as a GitHub Action May 27, 2020
@tklauser tklauser mentioned this issue May 28, 2020
Merged
tklauser added a commit that referenced this issue May 28, 2020
@tklauser
make: move lock and rand checks to precheck target
5e22b16 
There's no reason to wait with running contrib/scripts/lock-check.sh and
contrib/scripts/rand-check.sh until after the build (i.e. the postcheck
target) as they are using only `grep` to check. Move them to the
precheck target so these errors are caught earlier.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
tklauser added a commit that referenced this issue May 28, 2020
@tklauser
travis: run build checks and complete build
4ac4e01 
This will allow various issues before actually running the tests in our
CI. Use `-j 2` since Travis CI usually runs on 2 cores and some parts of
the build benefit from that.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
tklauser added a commit that referenced this issue May 28, 2020
@tklauser
make: move lock and rand checks to precheck target
47f165f 
There's no reason to wait with running contrib/scripts/lock-check.sh and
contrib/scripts/rand-check.sh until after the build (i.e. the postcheck
target) as they are using only `grep` to check. Move them to the
precheck target so these errors are caught earlier.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
tklauser added a commit that referenced this issue May 28, 2020
@tklauser
travis: run build checks and complete build
0d6fdc0 
This will allow various issues before actually running the tests in our
CI. Use `-j 2` since Travis CI usually runs on 2 cores and some parts of
the build benefit from that.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
aanm pushed a commit that referenced this issue May 29, 2020
@tklauser @aanm
make: move lock and rand checks to precheck target
610a501 
There's no reason to wait with running contrib/scripts/lock-check.sh and
contrib/scripts/rand-check.sh until after the build (i.e. the postcheck
target) as they are using only `grep` to check. Move them to the
precheck target so these errors are caught earlier.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
aanm pushed a commit that referenced this issue May 29, 2020
@tklauser @aanm
travis: run build checks and complete build
a6ca5d5 
This will allow various issues before actually running the tests in our
CI. Use `-j 2` since Travis CI usually runs on 2 cores and some parts of
the build benefit from that.

Updates #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@stale
Copy link

stale bot commented Jul 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 27, 2020
@pchaigno pchaigno removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jul 27, 2020
@tklauser tklauser self-assigned this Jul 28, 2020
@tklauser tklauser mentioned this issue Jul 28, 2020
Merged
tklauser added a commit that referenced this issue Jul 28, 2020
@tklauser
.github: run Go prechecks as GitHub action
2f0d40e 
Run `make precheck` as a GitHub action. This checks various formatting
and package usage issues in Go code.

Fixes #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
aanm pushed a commit that referenced this issue Jul 28, 2020
@tklauser @aanm
.github: run Go prechecks as GitHub action
8d56d06 
Run `make precheck` as a GitHub action. This checks various formatting
and package usage issues in Go code.

Fixes #11730

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tklauser tklauser

Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ci/flake This is a known failure that occurs in the tree. Please investigate me! good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

.github: run Go prechecks as GitHub action cilium/cilium
4 participants
@tklauser @joestringer @pchaigno @christarazi