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

✨ refactor and update Makefile #5806

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

Prajyot-Parab
Copy link
Contributor

Signed-off-by: Prajyot-Parab [email protected]

What this PR does / why we need it:

  1. Standardised target names for all the components.
  2. Consistent documentation across all the targets.
  3. Added dedicated targets for missing verify and hack/tools components.
  4. Re-shuffled order, grouped targets for easy reading, understanding and maintenance.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #5574

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 7, 2021

CLA Signed

The committers are authorized under a signed CLA.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 7, 2021
@k8s-ci-robot
Copy link
Contributor

Welcome @Prajyot-Parab!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 7, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @Prajyot-Parab. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 7, 2021
@sbueringer
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 7, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2021
@stmcginnis
Copy link
Contributor

/lgtm

Still need CLA signed.

Help output looks a lot better organized now:

Usage:
  make <target>

Targets:

generate:
  generate                                       Run all generate-manifests-*, generate-go-deepcopy-* and generate-go-conversions-* targets
  generate-manifests                             Run all generate-manifests-* targets
  generate-manifests-core                        Generate manifests e.g. CRD, RBAC etc. for core
  generate-manifests-kubeadm-bootstrap           Generate manifests e.g. CRD, RBAC etc. for kubeadm bootstrap
  generate-manifests-kubeadm-control-plane       Generate manifests e.g. CRD, RBAC etc. for kubeadm control plane
  generate-go-deepcopy                           Run all generate-go-deepcopy-* targets
  generate-go-deepcopy-core                      Generate deepcopy go code for core
  generate-go-deepcopy-kubeadm-bootstrap         Generate deepcopy go code for kubeadm bootstrap
  generate-go-deepcopy-kubeadm-control-plane     Generate deepcopy go code for kubeadm control plane
  generate-go-conversions                        Run all generate-go-conversions-* targets
  generate-go-conversions-core                   Generate conversions go code for core
  generate-go-conversions-kubeadm-bootstrap      Generate conversions go code for kubeadm bootstrap
  generate-go-conversions-kubeadm-control-plane  Generate conversions go code for kubeadm control plane
  diagrams                                       Generate diagrams for *.plantuml files

lint and verify:
  modules                                        Run go mod tidy to ensure modules are up to date
  lint                                           Lint the codebase
  lint-fix                                       Lint the codebase and run auto-fixers if supported by the linter
  apidiff                                        Check for API differences
  format-tiltfile                                Format the Tiltfile
  verify                                         Run all verify-* targets
  verify-modules                                 Verify go modules are up to date
  verify-gen                                     Verfiy go generated files are up to date
  verify-conversions                             Verifies API expected conversion are in place
  verify-boilerplate                             Verify boilerplate text exists in each file
  verify-shellcheck                              Verify shell files
  verify-tiltfile                                Verify Tiltfile format

build:
  clusterctl                                     Build the clusterctl binary
  managers                                       Run all manager-* targets
  manager-core                                   Build the core manager binary into the ./bin folder
  manager-kubeadm-bootstrap                      Build the kubeadm bootstrap manager binary into the ./bin folder
  manager-kubeadm-control-plane                  Build the kubeadm control plane manager binary into the ./bin folder
  docker-build                                   Run docker-build-core and all docker-build-kubeadm-* targets
  docker-build-all                               Build all the architecture docker images
  docker-build-core                              Build the docker image for core controller manager
  docker-build-kubeadm-bootstrap                 Build the docker image for kubeadm bootstrap controller manager
  docker-build-kubeadm-control-plane             Build the docker image for kubeadm control plane controller manager
  e2e-framework                                  Builds the CAPI e2e framework

test:
  test                                           Run all the tests
  test-e2e                                       Run the e2e tests
  test-junit                                     Run tests with verbose setting and generate a junit report
  test-cover                                     Run tests with code coverage and code generate reports
  test-verbose                                   Run tests with verbose settings
  serve-book                                     Build and serve the book with live-reloading enabled
  kind-cluster                                   Create a new kind cluster designed for testing with Tilt
  docker-build-e2e                               Rebuild all Cluster API provider images to be used in the e2e tests

release:
  release                                        Build and push container images using the latest git tag for the commit
  release-manifests                              Build the manifests to publish with a release
  release-manifests-dev                          Build the development manifests and copies them in the release folder
  release-binaries                               Build the binaries to publish with a release
  release-staging                                Build and push container images to the staging bucket
  release-staging-nightly                        Tag and push container images to the staging bucket. Example image tag: cluster-api-controller:nightly_main_20210121
  release-alias-tag                              Add the tag to the last build tag
  docker-push                                    Push the docker images
  docker-push-all                                Push all the architecture docker images
  docker-push-manifest-core                      Push the multiarch manifest for the core docker images
  docker-push-manifest-kubeadm-bootstrap         Push the multiarch manifest for the the kubeadm bootstrap docker images
  docker-push-manifest-kubeadm-control-plane     Push the multiarch manifest for the the kubeadm control plane docker images

clean:
  clean                                          Remove all generated files
  clean-bin                                      Remove all generated binaries
  clean-book                                     Remove all generated GitBook files
  clean-release                                  Remove the release folder
  clean-release-git                              Restores the git files usually modified during a release
  clean-generated-conversions                    Remove files generated by conversion-gen from the mentioned dirs. Example SRC_DIRS="./api/v1alpha4"

hack/tools:
  controller-gen                                 Build a local copy of controller-gen
  conversion-gen                                 Build a local copy of conversion-gen
  conversion-verifier                            Build a local copy of conversion-verifier
  gotestsum                                      Build a local copy of gotestsum
  go-apidiff                                     Build a local copy of apidiff
  golangci-lint                                  Build a local copy of golangci-lint
  envsubst                                       Build a local copy of envsubst
  kustomize                                      Build a local copy of kustomize
  setup-envtest                                  Build a local copy of setup-envtest
  yq                                             Build a local copy of yq
  kpromo                                         Build a local copy of kpromo

Current output is just one ungrouped list of all targets.

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 13, 2021
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 14, 2021
@Prajyot-Parab
Copy link
Contributor Author

Prajyot-Parab commented Dec 14, 2021

@stmcginnis Thanks for the review, I already have cla/linuxfoundation signed and label for same is already added. For EasyCLA I was told its a non-blocking and currently in testing phase, so they are reluctant to add my ID there until it is in stable phase. (I can still try asking them, if that will be a blocker in merging this PR)

^^^ CC- @fabriziopandini

Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

@Prajyot-Parab many thanks for picking up this work, I really appreciated it!
Just a couple of nits from my side, but overall changes are ok

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated
cd $(TOOLS_DIR); go mod tidy
cd $(TEST_DIR); go mod tidy
.PHONY: verify-book-links
verify-book-links:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
verify-book-links:
verify-book-links: ## Verify book links

Copy link
Member

Choose a reason for hiding this comment

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

@vincepri @CecileRobertMichon do you remember why we stopped to run this as part of make verify? Should we add a comment to track this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@WillsonHG
Copy link

/easycla

@fabriziopandini
Copy link
Member

@Prajyot-Parab thanks! this is a great first contribution!
/lgtm

@vincepri @CecileRobertMichon @enxebre @sbueringer PTAL when you have some spare time

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 16, 2021
Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

lgtm aside from minor comment on the test target

Signed-off-by: Prajyot-Parab <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 17, 2021
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 17, 2021
@CecileRobertMichon
Copy link
Contributor

@fabriziopandini should we merge this or did you want others to look at it?

This is going to cause conflicts with any PR that updates the Makefile so the sooner it merges the better

@fabriziopandini
Copy link
Member

@fabriziopandini should we merge this or did you want others to look at it?

This is going to cause conflicts with any PR that updates the Makefile so the sooner it merges the better

Let's get this moving, this PR greatly improves makefile usability

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2021
@k8s-ci-robot k8s-ci-robot merged commit 210b92d into kubernetes-sigs:main Dec 20, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.1 milestone Dec 20, 2021
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. 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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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.

Makefile targets re-shuffle
7 participants