Avoid watching all secrets in the cluster #1359
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This cache was unused since the req parameter was always nil.
This will allow us to choose between an informer and a kubernetes client. The current implementation is using an informer.
Instead of comparing nil secret with the given secret, it will return a meaningful error message.
Instead of using an informer, it now uses a k8s client. It avoids loading all secrets of the cluster in controller memory. Tests are still using an informer for convenience.
It will reduce the load on the k8s apiserver when receiving a lot of webhooks coming from the same project.
This is part of tektoncd#1271 - `names_match` violations still need to be dealt with. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
And fix tektoncd#1271 along the way by using the same scripting as in tektoncd/pipeline#4402 to ignore API rule violations for `name_match` which already exist. Any new mismatch will cause an error. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
Kubernetes vendoring is using klog v2 but knative is using v1. When knative configure the logging, it doesn't configure the underlying kubernetes logging. Fix tektoncd#1301
This will add `hack/update-reference-docs.sh` to `hack/update-codegen.sh`, resulting in `docs/triggers-api.md` being generated. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
add the ability to specify the service port for a eventlistener service. Default TLS Port 8443 is not set when service port is specified by the user. Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Changing the image location for triggers flow so that it can be rendered in tekton.dev/docs
Default HTTP client provided by go can cause leak, hangup and other issues because it has no sane timeout defined.
Adding metrics for counting EL, TT, CTB, clusterinteceptor and TB.
Define `targetURI` field in EL spec to send cloud event during trigger processing. FeatureFlag should also be `alpha`.
Send cloud event only when CloudEventURI is defined. Otherwise, logger will clog up the pod logs.
Update the tekton triggers build pipeline to build using golang 1.17.7 Controller images are built using ko. Updating the ko image in the publish task to one that includes golang 1.17.7 (golang:1.17.7-alpine3.15). Reference the ko image by sha instead of tag. This will require an extra step to update the image in future (plumbing first, triggers then) but it's best for attestations and also it allows updating the image on plumbing side and in the registry without direct side effect on triggers. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
Fix the param name in cel-eventlistener-interceptor.yaml example.
Bump the Go version to 1.17.8 in tekton/publish.yaml
Update Release Cheatsheet with instructions for chains and use release drafter pipeline without pipeline resources
tekton/publish task had a bug which lead to only interceptor images getting attestation.
Update CEL GO to v0.9.0 for fixing bugs and improving cel performance. https://github.com/google/cel-go/releases/tag/v0.9.0 https://github.com/google/cel-go/releases/tag/v0.8.0
CEL introduced support for heterogeneous equality and cross-numeric type comparisons. This means expressions like `json.value != null` will no longer error when the `json.value` is non-null. Additionally, JSON numbers which are usually `double` typed at runtime will successfully compare to signed and unsigned integer values as long as the values exist on the same point on a continuous number line The latest version of CEL reduces bootstrapping costs by >95%, with new environment instantiation dropping from 2.5ms to 100us. Evaluation performance also improves by 5-10% with list related macros improving by as much as 20x.
Wildcard/* should be quoted in yaml because * is used as anchor and aliases in yaml. So fix the documentation to put wildcard as "*".
As part of this, I also ran `go fmt -s -w cmd/ pkg/ test/ tekton/ hack/` to fix the build tags and then ran `./hack/update-codegen.sh` to regenerate the code. I also had to run `go mod tidy` and `go mod vendor` to fix up some inconsistent vendoring. Fixes tektoncd#1335 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
TriggersGroups has been moved to stable API from alpha api. It will be available by default and no changes in config will be required.
Remove the podtemplate struct from types.
This change uses non-tagged commits from `knative.dev/serving` and `knative.dev/eventing` because their `v0.31.0` releases require a minimum k8s cluster version of 1.22, and I wanted to avoid requiring that if at all possible. Instead, we depend on the latest commit in each before they changed to depending on a `knative.dev/pkg` commit newer than the one Pipeline depends on. Also add a Makefile, sync `.golangci.yml` with Pipeline's (other than requiring comments for exported functions/types/etc, which I'll come back and do in a followup), and fix some new-as-a-result linter issues. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@khrm: PR needs rebase. 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. |
|
@khrm: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
Closed this as I can directly push to Guillaume Rose's branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Replace the secret informer by a secret getter.
It implies that for each webhook an API call will be issued against the
k8s API to get the secret.
Previously, all secrets of the cluster were in the interceptor memory.
Before this change:
crictl stats reports 79.18MB with around 5k secrets of 2.5kB with kind.
After this change:
crictl stats reports 8.221MB
Proposition related to #1268
The other way to solve this would be to add a specific label on all secrets watched by the interceptor.
But it would introduce a breaking change between 2 releases.
Author - @guillaumerose
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes