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

use make #3007

Merged
merged 3 commits into from
Sep 24, 2021
Merged

use make #3007

merged 3 commits into from
Sep 24, 2021

Conversation

spiffxp
Copy link
Member

@spiffxp spiffxp commented Sep 24, 2021

I grew tired of recompiling protobuf to see if our YAML-based config was sorted

Adds the following human-facing targets:

  • merge: builds and installs the merge binary
  • config: creates a merged config using the merge binary
  • test: tests the merged config using go test
  • verify: runs hack/verify.sh
  • peribolos: clones a local copy of kubernetes/test-infra and installs
    the peribolos binary
  • deploy: runs periobolos to deploy the merged config if tests pass
  • clean: remove all intermediary configs and binaries

Overrideable env vars:

  • GITHUB_TOKEN_PATH: value for --github-token-path passed to peribolos
  • TEST_INFRA_PATH: set to location of a pre-existing clone of
    kubernetes/test-infra to avoid cloning a temporary copy

One way this differs from bazel is that we're not necessarily getting
the latest version of test-infra each time we run make deploy but I
don't foresee peribolos changing that frequently, and a make clean is
enough to get us back to "latest" state.

Adds the following human-facing targets:

- merge: builds and installs the merge binary
- config: creates a merged config using the merge binary
- test: tests the merged config using go test
- peribolos: clones a local copy of kubernetes/test-infra and installs
  the peribolos binary
- deploy: runs periobolos to deploy the merged config if tests pass
- clean: remove all intermediary configs and binaries

Overrideable env vars:

- GITHUB_TOKEN_PATH: value for --github-token-path passed to peribolos
- TEST_INFRA_PATH: set to location of a pre-existing clone of
  kubernetes/test-infra to avoid cloning a temporary copy

One way this differs from bazel is that we're not necessarily getting
the latest version of test-infra each time we run `make deploy` but I
don't foresee peribolos changing that frequently, and a `make clean` is
enough to get us back to "latest" state.
The verify-all.sh script ends up calling //hack:verify-all as defined in
kubernetes/repo-infra. Two of these checks are bazel-specific and can be
removed / ignored for porting:
- verify-bazel
- verify-deps

The other two make sense without bazel, so copy them over:
- verify-gofmt.sh (and update-gofmt)
- verify-boilerplate.sh

Then add a hack/verify.sh script (copied from kubernetes/k8s.io) that
calls all verify scripts (excluding verify-all.sh, to allow the
bazel-based tests to still run/pass)

Then add a verify target to the Makefile
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 24, 2021
@k8s-ci-robot k8s-ci-robot added area/github-management Issues or PRs related to GitHub Management subproject approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 24, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Sep 24, 2021

/cc @mrbobbytables @cblecker

@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/env python3
Copy link
Member Author

Choose a reason for hiding this comment

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

A note here: I didn't make any of these edits myself. I merely downloaded the latest copy of the script from kubernetes/repo-infra

@mrbobbytables
Copy link
Member

/lgtm
/hold
for @cblecker to weigh in 👍

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 24, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, spiffxp

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:

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

@mrbobbytables
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2021
@k8s-ci-robot k8s-ci-robot merged commit ad6818a into kubernetes:main Sep 24, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Sep 24, 2021
@spiffxp spiffxp deleted the use-make branch September 24, 2021 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/github-management Issues or PRs related to GitHub Management subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants