Setup local host & run end-to-end tests with a single command #525
Conversation
knative-prow-robot
added
kind/cleanup
knative-prow-robot
added
the
size/XXL
Codecov Report
@@ Coverage Diff @@
## main #525 +/- ##
==========================================
- Coverage 75.97% 75.58% -0.40%
==========================================
Files 46 47 +1
Lines 2781 2892 +111
==========================================
+ Hits 2113 2186 +73
- Misses 542 568 +26
- Partials 126 138 +12
Continue to review full report at Codecov.
|
n3wscott
left a comment
n3wscott
left a comment
There was a problem hiding this comment.
Makefiles do not work out of the box on windows and in Knative, we have never wanted to add makefiles to the project. Cc @evankanderson
|
thanks for the feedback @n3wscott !
If supporting Windows as a development platform is important, then something like GitHub Codespaces or Gitpod is the way to achieve that in my opinion. As an alternative, I think a more modern implementation of make like just may work too. I am not sure how to address the "in Knative" part.
If someone can show me a better alternative to |
|
I had a look at https://github.com/benmoss/knative-scripts and I think that all of it should live in this repository. I also think that bringing it up in the context of this PR is relevant since we could expose all those scripts as self-descriptive make targets. This is what that would look like in practice: Yes, a few targets are missing, but it's enough to show how this would actually work. |
|
Make works on Windows if you install it, it doesn't work on Linux without being installed either 😸 . Kubernetes uses Makefiles, they seem relatively uncontroversial to me. |
For example, running end-to-end tests requires setting up dependencies,
a local KiND cluster, installing operators, waiting for components to be
ready, and then invoking `go test` with many flags. The entire flow is
captured in .github/workflows/kind-e2e.yaml, so how do we run it
locally? I think that there are some scripts which try to set everything
up, but they are not discoverable, pretty sure they diverged from the CI
workflow by now, and are not as composable as the Make equivalent.
So how does it work? Type make and follow along:
build Build binaries with e2e tags
dep-update Update any dependency
e2e-setup Setup environment for e2e testing
env Configure shell env - eval "$(make env)" OR rm .env && make .env && source .env
k9s Terminal ncurses UI for K8s
test-e2e-broker Run Broker end-to-end tests
test-e2e-publish Run TestKoPublish end-to-end tests
test-e2e-source Run Source end-to-end tests
test-unit Run unit tests
To run end-to-end tests, which is what I had to do in the context of
knative-extensions#492 (this PR
is based on that one) I now only need to run the following:
# This only needs to run once:
make e2e-setup
make test-e2e
# I can run just a subset of tests with e.g.:
make test-e2e-broker
This iteration:
- only support Linux & Darwin systems (does not support Windows)
- only supports x84_64 (does not support latest Macs with ARM silicon)
- defaults to the minimum supported K8s version (adding support for
multiple K8s versions was out of scope)
This was only tested on Linux Ubuntu 20.04.3 LTS, and should be tested
on Darwin on Intel. Supporting Darwin on ARM should not be difficult,
but as I don't have access to one, I cannot test.
Signed-off-by: Gerhard Lazu <[email protected]>
We do not want to version-control these Signed-off-by: Gerhard Lazu <[email protected]>
Used in `make kind-cluster` Signed-off-by: Gerhard Lazu <[email protected]>
This comes in handy for checking out PRs, working with Actions, etc. Signed-off-by: Gerhard Lazu <[email protected]>
Allow versions to be changed just-in-time Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
After reading the first 6 chapters of Knative in Action, it became obvious to me that this CLI is essential for working with Knative. Signed-off-by: Gerhard Lazu <[email protected]>
knative-prow-robot
added
size/L
|
Now that #492 is merged, this is a focused changed. I have been using this for about 2 weeks and it works well. There is a single thing missing - a paragraph added to README - but otherwise this is ready to be merged. Try it out locally and let me know how well it works for you in practice. If this change is a net positive for the people that work with this repository every day, my proposal would be to merge it. If it doesn't work for you, capture your reasons - a sentence is OK - and let's move on. cc @ikvmw @xtreme-sameer-vohra |
knative-prow-robot
removed
the
do-not-merge/work-in-progress
On Windows on WSL with make 4.3 this fails. Not sure why it worked on Linux with make 4.2.1 as it's clearly wrong. This fixes it. Signed-off-by: Gerhard Lazu <[email protected]>
Variables declared with := get interpreted a single time while those with = get evaluated every time they are invoked. We want just once invocation. Signed-off-by: Gerhard Lazu <[email protected]>
That .env is always a gotcha... direnv doesn't make it better. Signed-off-by: Gerhard Lazu <[email protected]>
cardil
left a comment
cardil
left a comment
There was a problem hiding this comment.
-1 from me. Effectively merging couple bash scripts into one Makefile.
If we adopt makefile we should compose them from reusable parts to keep it manageable.
| GCLOUD_BIN := gcloud-$(GCLOUD_SDK_VERSION)-$(PLATFORM)-x86_64 | ||
| GCLOUD := $(LOCAL_BIN)/$(GCLOUD_BIN) | ||
| GCLOUD_SDK_FILE := google-cloud-sdk-$(GCLOUD_SDK_VERSION)-$(PLATFORM)-x86_64.tar.gz | ||
| GCLOUD_SDK_URL := https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/$(GCLOUD_SDK_FILE) |
There was a problem hiding this comment.
I don't like downloading tools directly from the network. I have them installed on my machine, securely. Without checking for consistency (signs, checksums) you are introducing a vulnerability into Knative supply chain.
There was a problem hiding this comment.
I definitely think that this area should be improved.
I also think the zero-trust approach promoted by chainguard.dev is the right way to go, and considering the people that are behind this idea - @mattmoor @vaikas @n3wscott - I think that they would agree with this too.
Given that we adopt the same approach in our existing GitHub Actions, and we suggest the same in the Knative development guide, I don't see the approach in this PR being worse. Having said that, I'm glad that this is on your radar and I also think that we should close this gap as soon as possible, but not part of this PR.
Makefile
Outdated
| $(KUBECTL) wait --for=condition=available deploy/eventing-controller --timeout=60s --namespace $(EVENTING_NAMESPACE) | ||
| $(KUBECTL) wait --for=condition=available deploy/eventing-webhook --timeout=60s --namespace $(EVENTING_NAMESPACE) | ||
|
|
||
| install: | $(KUBECONFIG) $(KO) install-knative-serving install-knative-eventing install-rabbitmq-cluster-operator install-rabbitmq-topology-operator ## Install local dev Knative Eventing RabbitMQ - manages all dependencies, including K8s components |
There was a problem hiding this comment.
This script is sooo long. Why will be maintaining this? There are 20-30 knative repos. Are we realy like to copy and paste this so many times, and change couple of things, that needs adjusting, in between?!?
There was a problem hiding this comment.
I understand that your perspective is the wider Knative ecosystem and I must agree that I have not thought about the big picture - my focus is to get this working in the context of this repository.
I recognise the tension between Make it work vs Make it right and while I welcome the second challenge, the focus is on small scope and trying to understand if this approach will even work.
I must emphasise that this is not a script, it's a collection of commands chained together. The distinction may seem insignificant, but it is an important mind shift for approaching Make the right way. This example illustrates Make approached from a different perspective which is valid for that context, but in my opinion it would be the wrong approach for Knative:
Thank you for the feedback! If you mean shell commands, then I agree. There are no function calls, includes, or libraries, it's just a bunch of commands that run selectively & sequentially.
I would keep this as simple as it can be, and see how well it works in practice for everyone on the team, across a few weeks, before investing more in this direction. I think seeing the Magefile alternative to this would be super valuable, so if you want to contribute that version, I would be all for it. I think there is a lot of benefit in that approach, but I don't have your context, and being able to compare both approaches side-by-side would really help me fill in the missing gaps. |
I like it @benmoss! https://github.com/knative-sandbox/eventing-rabbitmq/pull/525/files#r765968362 Signed-off-by: Gerhard Lazu <[email protected]>
This is something that @ikvmw was mentioning yesterday. Making this option visibile should help with WTF moments that originate in a stale cache. Signed-off-by: Gerhard Lazu <[email protected]>
The CI will continuously validate that everything is wired up correctly, and devs using this locally will have the confidence that it just works. Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
This is only necessary if pushing images to gcr.io, which is what *some* of us do locally, but not everyone. Signed-off-by: Gerhard Lazu <[email protected]>
Otherwise weird things happen, like KiND_CLUSTER_NAME vs KIND_CLUSTER_NAME. Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
|
For some reason, brokers and triggers are failing to reconcile correctly when running conformance tests via Has anyone seen this before? I suspect it's an environment variable not being propagated correctly, but it's possible that one of the deps changed versions. Investigating... |
|
Looks like your test-conformance-standalone is not installing the standalone broker |
Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
Thanks @benmoss for spotting it! knative-extensions#525 (comment) Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
|
The conformance tests as well as the e2e tests are passing locally, using the same versions that are failing in GitHub Actions. I think that we may hitting against the limit of the free GitHub-hosted runners. It may be that we are installing more things by default, or that we are moving through the stages quicker, but this is what the local runs look like: make.conformance-standalone.log I am running the above on a g6-dedicated-8, which translates to 8-cores of a AMD EPYC 7601 & 16GB DDR4. @benmoss should we keep this addition as dev-only, and solve the GitHub Actions (CI) issues separately? |
So that they are less likely to fail in GitHub Actions. Signed-off-by: Gerhard Lazu <[email protected]>
Signed-off-by: Gerhard Lazu <[email protected]>
knative-prow-robot
added
size/XXL
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benmoss, gvmw 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 |
knative-prow-robot
added
the
approved
6 participants
Changes
Makefileto make discovery and running of dev-related tasks easy and straightforward.github/workflows/kind-e2e.yaml.github/workflows/kind-conformance-standalone.yaml.github/workflows/kind-conformance.yamlI had to figure out how to setup my local environment for running end-to-end tests, and while there is a script in this repo that does the same thing, it is outdated, incomplete and difficult to find. While all these are solvable problems, the discovery and accessibility of portions of this script are more difficult to tackle. Multiple individual scripts that are wrapped in a single one with shared functions is a common way of solving this, but it takes very little for this to become hard to maintain and own by newcomers. Having used both approaches in multiple projects over the last decade, this is a better overall experience.
How do I use this?
Given a clean Linux host with
make, Golang & Docker Engine installed, this is how it works:Let's focus on the first time setup:
To check if this is setup correctly, let's check
makeautocompletion (assumes your shell is bash):Great! Let's install all the things:
kubectl,kindand then KiNDeventing-rabbitmq-e2ecluster with everything that is needed to run tests:How do I run end-to-end tests?
To run end-to-end tests, which is what I had to do in the context of #492 (this PR is based on that one), I now only need to run the following:
make test-e2e # I can run just a subset of tests with e.g.: make test-e2e-brokerHow do I run conformance tests?
👉🏻 @gvmw to do this next
What was in scope for this PR?
This was only tested on Linux Ubuntu 20.04.3 LTS, and should be tested on Darwin on Intel (I don't run Docker on Darwin). Supporting Darwin on ARM should not be difficult, but as I don't have access to one, I cannot test it.
By the way, there is a related Knative Proposal regarding this topic: Google Doc: Proposal Makefile for Knative
cc @benmoss @nader-ziada @n3wscott @salaboy