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

[v2] Add pre-commit and CI static checks #1025

Merged
merged 7 commits into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@
"-v", "keda-gomodcache:/go/pkg",
// Cache vscode exentsions installs and homedir
"-v", "keda-vscodecache:/root/.vscode-server",

// Mount docker socket for docker builds
"-v", "/var/run/docker.sock:/var/run/docker.sock",

"--cap-add=SYS_PTRACE", "--security-opt", "seccomp=unconfined"
],

// Use 'settings' to set *default* container specific settings.json values on container create.
// Use 'settings' to set *default* container specific settings.json values on container create.
// You can edit these settings after create using File > Preferences > Settings > Remote.
"settings": {
"settings": {
"terminal.integrated.shell.linux": "/bin/bash",
"go.gopath": "/go"
},
Expand All @@ -31,4 +31,4 @@
"extensions": [
"ms-vscode.go"
]
}
}
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence
* @ahmelsayed @zroubalik
* @ahmelsayed @zroubalik
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/Bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ A clear and concise description of what the bug is.
- **KEDA Version:** *Please elaborate*
- **Platform & Version:** *Please elaborate*
- **Kubernetes Version:** *Please elaborate*
- **Scaler(s):** *Please elaborate*
- **Scaler(s):** *Please elaborate*
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/Suggest_scaler.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ A clear and concise description of what scaler you'd like to use and how you'd w

- **Scaler Source:** *Please elaborate*
- **How do you want to scale:** *Please elaborate*
- **Authentication:** *Please elaborate*
- **Authentication:** *Please elaborate*
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ blank_issues_enabled: false
contact_links:
- name: Ask a question or get support
url: https://github.com/kedacore/keda/discussions/new
about: Ask a question or request support for using KEDA
about: Ask a question or request support for using KEDA
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!-- Thank you for contributing!

Read more about how you can contribute in our contribution guide:
https://github.com/kedacore/keda/blob/master/CONTRIBUTING.md
-->
Expand Down
25 changes: 25 additions & 0 deletions .github/workflows/code-quality.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: Code Quality
on:
push:
branches:
- master
- v2
pull_request:
branches:
- master
- v2
jobs:
statics:
name: Static checks
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v1
- uses: actions/setup-go@v2-beta
with:
go-version: 1.15
- name: install go-lint
run: |
go get -u golang.org/x/lint/golint
export PATH=$PATH:$(go list -f {{.Target}} golang.org/x/lint/golint)
- uses: pre-commit/action@v1.0.1
2 changes: 1 addition & 1 deletion .github/workflows/pr-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:

- name: Verify Generated clientset is up to date
run: make clientset-verify

- name: Build
run: make build

Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -346,4 +346,4 @@ vendor
cover.out

# GO debug binary
cmd/manager/debug.test
cmd/manager/debug.test
77 changes: 77 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
default_stages: [commit, push]
minimum_pre_commit_version: "1.20.0"
repos:
- repo: git://github.com/dnephin/pre-commit-golang
rev: master
hooks:
- id: go-fmt
- id: go-lint
exclude: |
(?x)(
.*zz_generated.*|
^api/v1alpha1/condition_types\.go$|
^api/v1alpha1/groupversion_info\.go$|
^api/v1alpha1/gvkr_types\.go$|
^api/v1alpha1/scaledjob_types\.go$|
^api/v1alpha1/scaledobject_types\.go$|
^api/v1alpha1/triggerauthentication_types\.go$|
^controllers/scaledjob_controller\.go$|
^controllers/scaledobject_controller\.go$|
^controllers/util/status\.go$|
^controllers/util/string_lists\.go$|
^hack/tools\.go$|
^pkg/scalers/artemis_scaler\.go$|
^pkg/scalers/azure/azure_aad_podidentity\.go$|
^pkg/scalers/azure/azure_eventhub\.go$|
^pkg/scalers/azure/azure_monitor\.go$|
^pkg/scalers/azure/azure_queue\.go$|
^pkg/scalers/azure/azure_storage\.go$|
^pkg/scalers/azure_eventhub_scaler\.go$|
^pkg/scalers/azure_queue_scaler\.go$|
^pkg/scalers/azure_servicebus_scaler\.go$|
^pkg/scalers/cron_scaler\.go$|
^pkg/scalers/external_scaler\.go$|
^pkg/scalers/kafka_scram_client\.go$|
^pkg/scalers/liiklus_scaler\.go$|
^pkg/scalers/postgresql_scaler\.go$|
^pkg/scalers/rabbitmq_scaler\.go$|
^pkg/scalers/rabbitmq_scaler_test\.go$|
^pkg/scalers/scaler\.go$|
^pkg/scaling/executor/scale_executor\.go$|
^pkg/scaling/resolver/hashicorpvault_handler\.go$|
^pkg/scaling/resolver/scale_resolvers\.go$|
^pkg/util/gvkr\.go$|
^pkg/util/k8sversion\.go$|
^pkg/util/normalize_string\.go$|
^version/version\.go$
)
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: trailing-whitespace
- id: detect-private-key
- id: end-of-file-fixer
- id: check-merge-conflict
- id: mixed-line-ending
- repo: https://github.com/thlorenz/doctoc.git
rev: v1.4.0
hooks:
- id: doctoc
name: Add TOC for md files
files: ^README\.md$|^CONTRIBUTING\.md$
args:
- "--maxlevel"
- "2"
- repo: local
hooks:
- id: language-matters
language: pygrep
name: Check for language that we do not accept as community
description: Please use "deny_list" or "allow_list" instead.
entry: "(?i)(black|white)[_-]?(list|List)"
pass_filenames: true
- id: sort-scalers
name: Check if scalers are sorted in scaler_handler.go
language: system
entry: "bash tools/sort_scalers.sh"
files: .*scale_handler\.go$
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
38 changes: 38 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ Thanks for helping make KEDA better 😍.

There are many areas we can use contributions - ranging from code, documentation, feature proposals, issue triage, samples, and content creation.

<!-- START doctoc generated TOC please keep comment here to allow auto update -->
Copy link
Member

Choose a reason for hiding this comment

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

That's great, but how does this work? Should add some docs on 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.

I would say that's enough to google doctoc to find out more information. Also not sure where to put such docs

Copy link

Choose a reason for hiding this comment

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

Yeah. And wit pre-commits you basically do not have to worry about it - just install pre-commit and you forget about it..

Copy link
Member

Choose a reason for hiding this comment

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

That's a bit of an odd argument as that would mean we don't have to doc the pre-commit thingy but it's ok.

Yeah. And wit pre-commits you basically do not have to worry about it - just install pre-commit and you forget about it..

We tend to make it easy for folks to contribute so you cannot assume everybody has pre-commit installed, that's why we have a CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We tend to make it easy for folks to contribute so you cannot assume everybody has pre-commit installed, that's why we have a CI.

As we have totally lightweight pre-commit setup we should probably encourage people to use them. It's good to teach people how to automate stuff and focus on the most important things like the code itself and documentation. However, that's only my personal opinion.

Copy link
Member

Choose a reason for hiding this comment

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

That's definitely the case and I agree, but we can't expect everyone to use it.

Copy link

Choose a reason for hiding this comment

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

We also have it integrated on the CI, so if someone does not use pre-commit locally, they will get a readable diff on the CI that will show what changes should be made.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that was my point on #1025 (comment) indeed

Copy link

@potiuk potiuk Aug 25, 2020

Choose a reason for hiding this comment

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

Yeah. Pre-commits are working like a charm on Apache Airflow for > 1.5 year now and we have plenty of pre-commits running/ You can see it working here for example: https://github.com/apache/airflow/runs/1026196376?check_suite_focus=true#step:7:108 - it integrates very well with CI. you simply run pre-commit run --all-files in the CI and you get the very same checks executed in CI as locally.

And as @mik-laj mentioned - with --show-diff-on-failure flag it will even show you what modification would be made if you run pre-commit locally. In case it fails in CI it will even tell you what to do 'pip install pre-commit' and then pre-commit run <id of pre-commit that failed>. But you still can fix it manually if you prefer (though a lot of pre-commit checks can fix things for you automatically (like end-of-file checking, sorting import in python etc.) and then the only thing you have to do you have to commit modified files. And you have literally hundreds of hooks that you can start using.

So you do not have to even install it as pre-commit (though it is super-easy pre-commit install will do the job after installit it with pip. The nice thing about it is that you simply forget about it once you install - it will automatically run all the precommits that were added by others since and you will actually fix the problems even before others see them.

I do not see any drawbacks of using pre-commit to be honest if your project's goal is to keep some consistency, it's the perfect tool for the job IMHO.

Copy link
Member

Choose a reason for hiding this comment

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

Don't get me wrong, I'm not saying it's a bad thing or so; I was just saying that it's fine and the CI will cover it but we shouldn't force people to use it.

I'm not saying you are, it was just a general remark :) This escalated quickly.

<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
**Table of contents**

- [Getting Help](#getting-help)
- [Contributing Scalers](#contributing-scalers)
- [Including Documentation Changes](#including-documentation-changes)
- [Creating and building a local environment](#creating-and-building-a-local-environment)
- [Developer Certificate of Origin: Signing your work](#developer-certificate-of-origin-signing-your-work)
- [Code Quality](#code-quality)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

## Getting Help

If you have a question about KEDA or how best to contribute, the [#KEDA](https://kubernetes.slack.com/archives/CKZJ36A5D) channel on the Kubernetes slack channel ([get an invite if you don't have one already](https://slack.k8s.io/)) is a good place to start. We also have regular [community stand-ups](https://github.com/kedacore/keda#community-standup) to track ongoing work and discuss areas of contribution. For any issues with the product you can [create an issue](https://github.com/kedacore/keda/issues/new) in this repo.
Expand Down Expand Up @@ -68,3 +81,28 @@ git add -A
git commit -sm "one commit on <branch-name>"
git push --force
```

## Code Quality

This project is using [pre-commits](https://pre-commit.com) to ensure the quality of the code. Every change
turbaszek marked this conversation as resolved.
Show resolved Hide resolved
is checked on CI and if it does not pass the tests it cannot be accepted. If you want to check locally then
you should install Python3.6 or newer together and run:
```bash
pip install pre-commit
# or
brew install pre-commit
```
For more installation options visit the [pre-commits](https://pre-commit.com).

To turn on pre-commit checks for commit operations in git, run:
```bash
pre-commit install
```
To run all checks on your staged files, run:
```bash
pre-commit run
```
To run all checks on all files, run:
```bash
pre-commit run --all-files
```
2 changes: 1 addition & 1 deletion Dockerfile.adapter
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ COPY --from=builder /workspace/bin/keda-adapter .
USER nonroot:nonroot


ENTRYPOINT ["/keda-adapter", "--secure-port=6443", "--logtostderr=true", "--v=0"]
ENTRYPOINT ["/keda-adapter", "--secure-port=6443", "--logtostderr=true", "--v=0"]
2 changes: 1 addition & 1 deletion GOVERNANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ maintainers over the course of a one week voting period. At the end of the week,
votes are counted and a pull request is made on the repo adding the new
maintainer to the [MAINTAINERS](MAINTAINERS.md) file.

Individuals interested in becoming maintainers may submit an [issue](https://github.com/kedacore/keda/issues/new)
Individuals interested in becoming maintainers may submit an [issue](https://github.com/kedacore/keda/issues/new)
stating their interest. Existing maintainers can choose if they would
like to nominate these individuals to be a maintainer following the process
above.
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,4 @@
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
limitations under the License.
2 changes: 1 addition & 1 deletion MAINTAINERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@
| -------------------- | --------------------------------------------- | ----------- |
| Aarthi Saravanakumar | [Aarthisk](https://github.com/Aarthisk) | Microsoft |
| Yaron Schneider | [yaron2](https://github.com/yaron2) | Microsoft |
| Ben Browning | [bbrowning](https://github.com/bbrowning) | Red Hat |
| Ben Browning | [bbrowning](https://github.com/bbrowning) | Red Hat |
56 changes: 41 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,37 @@ Make sure to remove previous KEDA (including CRD) from the cluster. Switch to th
<a href="https://bestpractices.coreinfrastructure.org/projects/3791"><img src="https://bestpractices.coreinfrastructure.org/projects/3791/badge"></a>
<a href="https://twitter.com/kedaorg"><img src="https://img.shields.io/twitter/follow/kedaorg?style=social" alt="Twitter"></a></p>

KEDA allows for fine-grained autoscaling (including to/from zero) for event driven Kubernetes workloads. KEDA serves
as a Kubernetes Metrics Server and allows users to define autoscaling rules using a dedicated Kubernetes custom
KEDA allows for fine-grained autoscaling (including to/from zero) for event driven Kubernetes workloads. KEDA serves
as a Kubernetes Metrics Server and allows users to define autoscaling rules using a dedicated Kubernetes custom
resource definition.

KEDA can run on both the cloud and the edge, integrates natively with Kubernetes components such as the Horizontal
KEDA can run on both the cloud and the edge, integrates natively with Kubernetes components such as the Horizontal
Pod Autoscaler, and has no external dependencies.

We are a Cloud Native Computing Foundation (CNCF) sandbox project.
<img src="https://raw.githubusercontent.com/kedacore/keda/master/images/logo-cncf.svg" height="75px">


<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
**Table of contents**

- [Getting started](#getting-started)
- [Deploying KEDA](#deploying-keda)
- [Documentation](#documentation)
- [FAQ](#faq)
- [Samples](#samples)
- [Releases](#releases)
- [Contributing](#contributing)
- [Community](#community)
- [Building: Quick start with Visual Studio Code Remote - Containers](#building-quick-start-with-visual-studio-code-remote---containers)
- [Building: Locally directly](#building-locally-directly)
- [Deploying: Custom KEDA locally outside cluster](#deploying-custom-keda-locally-outside-cluster)
- [Deploying: Custom KEDA as an image](#deploying-custom-keda-as-an-image)
- [Setting log levels](#setting-log-levels)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->


## Getting started

* [QuickStart - RabbitMQ and Go](https://github.com/kedacore/sample-go-rabbitmq)
Expand Down Expand Up @@ -62,19 +83,19 @@ You can find Contributing guide [here](./CONTRIBUTING.md)

If interested in contributing or participating in the direction of KEDA, you can join our community meetings.

* **Meeting time:** Bi-weekly Thurs 16:00 UTC (does follow US daylight savings).
* **Meeting time:** Bi-weekly Thurs 16:00 UTC (does follow US daylight savings).
([Subscribe to Google Agenda](https://calendar.google.com/calendar?cid=bjE0bjJtNWM0MHVmam1ob2ExcTgwdXVkOThAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ) |
[Convert to your timezone](https://www.thetimezoneconverter.com/?t=04%3A00%20pm&tz=UTC))
* **Zoom link:** [https://zoom.us/j/150360492 ](https://zoom.us/j/150360492 )
* **Meeting agenda:** [https://hackmd.io/s/r127ErYiN](https://hackmd.io/s/r127ErYiN)

Just want to learn or chat about KEDA? Feel free to join the conversation in
Just want to learn or chat about KEDA? Feel free to join the conversation in
**[#KEDA](https://kubernetes.slack.com/messages/CKZJ36A5D)** on the **[Kubernetes Slack](https://slack.k8s.io/)**!

## Building: Quick start with [Visual Studio Code Remote - Containers](https://code.visualstudio.com/docs/remote/containers)

This helps you pull and build quickly - dev containers launch the project inside a container with all the tooling
required for a consistent and seamless developer experience.
This helps you pull and build quickly - dev containers launch the project inside a container with all the tooling
required for a consistent and seamless developer experience.

This means you don't have to install and configure your dev environment as the container handles this for you.

Expand All @@ -92,11 +113,11 @@ code .
Once VSCode launches run `CTRL+SHIFT+P -> Remote-Containers: Reopen in container` and then use the integrated
terminal to run:

```bash
```bash
make build
```

> Note: The first time you run the container it will take some time to build and install the tooling. The image
> Note: The first time you run the container it will take some time to build and install the tooling. The image
> will be cached so this is only required the first time.

## Building: Locally directly
Expand Down Expand Up @@ -127,9 +148,14 @@ go env -w GOPROXY=https://proxy.golang.org,direct GOSUMDB=sum.golang.org
```

## Deploying: Custom KEDA locally outside cluster
<<<<<<< HEAD
The Operator SDK framework allows you to run the operator/controller locally outside the cluster without
a need of building an image. This should help during development/debugging of KEDA Operator or Scalers.
> Note: This approach works only on Linux or macOS.
a need of building an image. This should help during development/debugging of KEDA Operator or Scalers.
> Note: This approach works only on Linux or macOS.
=======
The Operator SDK framework allows you to run the operator/controller locally outside the cluster without a need of building an image. This should help during development/debugging of KEDA Operator or Scalers.
> Note: This approach works only on Linux or macOS.
>>>>>>> Add pre-commit and CI static checks
turbaszek marked this conversation as resolved.
Show resolved Hide resolved

To have fully operational KEDA we need to deploy Metrics Server first.

Expand All @@ -145,11 +171,11 @@ To have fully operational KEDA we need to deploy Metrics Server first.
and change the operator log level via `--zap-log-level=` if needed
```bash
make run ARGS="--zap-log-level=debug"
```
```

## Deploying: Custom KEDA as an image

If you want to change KEDA's behaviour, or if you have created a new scaler (more docs on this to come) and you want
If you want to change KEDA's behaviour, or if you have created a new scaler (more docs on this to come) and you want
to deploy it as part of KEDA. Do the following:

1. Make your change in the code.
Expand All @@ -162,7 +188,7 @@ to deploy it as part of KEDA. Do the following:
```bash
IMAGE_REPO=johndoe make deploy
```
4. Once the keda pods are up, check the logs to verify everything running ok, eg:
4. Once the keda pods are up, check the logs to verify everything running ok, eg:
```bash
kubectl get pods --no-headers -n keda | awk '{print $1}' | grep keda-operator | xargs kubectl -n keda logs -f

Expand Down
Loading