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.

Sign up for GitHub

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

Jump to bottom

[Chore] Fix tests migrated to Chainsaw #2650

Merged
merged 1 commit into from
Feb 21, 2024
Merged

[Chore] Fix tests migrated to Chainsaw #2650

merged 1 commit into from
Feb 21, 2024

Conversation

IshwarKanse
Copy link
Contributor

Description:

  • Update tests/e2e-openshift/Dockerfile to install Chainsaw.
  • Fix the tests migrated to Chainsaw.

@IshwarKanse IshwarKanse requested a review from a team February 19, 2024 13:31
@IshwarKanse IshwarKanse marked this pull request as draft February 19, 2024 13:32
@IshwarKanse IshwarKanse force-pushed the fix branch 2 times, most recently from 70e60ae to 68911d0 Compare February 19, 2024 15:11
@IshwarKanse IshwarKanse marked this pull request as ready for review February 19, 2024 15:21
@IshwarKanse
Copy link
Contributor Author

IshwarKanse commented Feb 19, 2024 •
edited
Loading 

Tested on OpenShift 4.14:
--- PASS: chainsaw (0.00s)
    --- PASS: chainsaw/smoke-pod-labels (12.40s)
    --- PASS: chainsaw/autoscale (24.61s)
    --- PASS: chainsaw/smoke-targetallocator (20.21s)
    --- PASS: chainsaw/smoke-statefulset (10.73s)
    --- PASS: chainsaw/smoke-simplest (9.95s)
    --- PASS: chainsaw/statefulset-features (42.78s)
    --- PASS: chainsaw/smoke-shareprocessnamespace (9.19s)
    --- PASS: chainsaw/targetallocator-prometheuscr (62.27s)
    --- PASS: chainsaw/smoke-restarting-deployment (12.04s)
    --- PASS: chainsaw/smoke-sidecar-other-namespace (44.39s)
    --- PASS: chainsaw/smoke-sidecar (43.76s)
    --- PASS: chainsaw/instrumentation-python (56.04s)
    --- PASS: chainsaw/create-sm-prometheus-exporters (46.16s)
    --- PASS: chainsaw/target-allocator (10.52s)
    --- PASS: chainsaw/targetallocator-features (68.10s)
    --- PASS: chainsaw/pdb (9.12s)
    --- PASS: chainsaw/route (15.38s)
    --- PASS: chainsaw/targetallocator-kubernetessd (104.58s)
    --- PASS: chainsaw/create-pm-prometheus-exporters (59.58s)
    --- PASS: chainsaw/monitoring (35.83s)
    --- PASS: chainsaw/otlp-metrics-traces (55.93s)
    --- PASS: chainsaw/opampbridge (13.11s)
    --- PASS: chainsaw/multi-cluster (54.55s)
    --- PASS: chainsaw/multiple-configmaps (12.31s)
    --- PASS: chainsaw/smoke-pod-annotations (9.62s)
    --- PASS: chainsaw/smoke-init-containers (16.86s)
    --- PASS: chainsaw/instrumentation-python-multicontainer (42.37s)
    --- PASS: chainsaw/instrumentation-sdk (54.71s)
    --- PASS: chainsaw/smoke-daemonset (11.12s)
    --- PASS: chainsaw/instrumentation-java-multicontainer (16.66s)
    --- PASS: chainsaw/prometheus-config-validation (26.93s)
    --- PASS: chainsaw/instrumentation-nodejs-multicontainer (21.12s)
    --- PASS: chainsaw/instrumentation-nodejs (16.80s)
    --- PASS: chainsaw/instrumentation-nginx-contnr-secctx (17.32s)
    --- PASS: chainsaw/instrumentation-nginx-multicontainer (19.67s)
    --- PASS: chainsaw/instrumentation-nginx (18.10s)
    --- PASS: chainsaw/instrumentation-java-other-ns (23.06s)
    --- PASS: chainsaw/managed-reconcile (15.32s)
    --- PASS: chainsaw/kafka (160.94s)
    --- PASS: chainsaw/ingress (10.97s)
    --- PASS: chainsaw/daemonset-features (17.13s)
    --- PASS: chainsaw/instrumentation-java (15.33s)
    --- PASS: chainsaw/instrumentation-dotnet-multicontainer (17.07s)
    --- SKIP: chainsaw/instrumentation-apache-multicontainer (0.00s)
    --- PASS: chainsaw/ingress-subdomains (51.95s)
    --- PASS: chainsaw/instrumentation-dotnet (14.54s)
    --- PASS: chainsaw/instrumentation-dotnet-musl (14.99s)
    --- PASS: chainsaw/instrumentation-apache-httpd (16.45s)
    --- PASS: chainsaw/instrumentation-go (56.64s)
PASS
Tests Summary...
- Passed  tests 48
- Failed  tests 0
- Skipped tests 1
Done.

--- PASS: chainsaw (0.00s)
    --- PASS: chainsaw/instrumentation-multi-no-containers (40.33s)
    --- PASS: chainsaw/instrumentation-single-instr-first-container (42.34s)
    --- PASS: chainsaw/instrumentation-multi-multicontainer (61.88s)
PASS
Tests Summary...
- Passed  tests 3
- Failed  tests 0
- Skipped tests 0
Done.

swiatekm
swiatekm approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm not an OpenShift expert, would appreciate review from another maintainer of these tests.

@IshwarKanse
Copy link
Contributor Author

@frzifus Need your approval on the PR.

@frzifus
Copy link
Member

frzifus commented Feb 20, 2024

Seems the e2e test is failing:

 === NAME  chainsaw/targetallocator-prometheuscr
    l.go:53: ���������| 15:51:52 | targetallocator-prometheuscr | step-00  | APPLY     | ERROR | rbac.authorization.k8s.io/v1/ClusterRoleBinding @ (join('-', ['ta', $namespace]))
        === ERROR
        metadata.name: Internal error: SyntaxError: Unexpected token at the end of the expression: TOKStringLiteral
=== NAME  chainsaw/targetallocator-features
    l.go:53: ���������| 15:51:52 | targetallocator-features | step-00  | APPLY     | ERROR | rbac.authorization.k8s.io/v1/ClusterRoleBinding @ (join('-', ['default-view', $namespace]))
        === ERROR
        metadata.name: Internal error: SyntaxError: Unexpected token at the end of the expression: TOKStringLiteral

@eddycharly
Copy link
Contributor

It looks like the templating is not applied 🤔

@eddycharly
Copy link
Contributor

@IshwarKanse one workaround is to set the template field at the test level.

spec:
  template: true
  steps:
  # ...

I'm investigating...

@eddycharly
Copy link
Contributor

Nope, that's a flake, i can reproduce it :(

@frzifus
Copy link
Member

frzifus commented Feb 20, 2024

I triggered a 3rd re-run. Lets see, but if thats not a behavior on main I think we should fix it first.

cc @jaronoff97

@eddycharly
Copy link
Contributor

It’s definitely a bug in chainsaw. Something goes wrong with parsing expressions concurrently.

@eddycharly
Copy link
Contributor

Ok, found it, will be fixed in the next version (hopefully tomorrow).

@eddycharly
Copy link
Contributor

For reference kyverno/kyverno-json#312 🙈

@eddycharly
Copy link
Contributor

eddycharly commented Feb 20, 2024 •
edited
Loading

@IshwarKanse i fixed the issue, i'm going to cut v0.1.5-beta.1 if you want to try it (one PR is missing and will work on the docs, v0.1.5 should be available tomorrow).

https://github.com/kyverno/chainsaw/actions/runs/7979571759

@IshwarKanse IshwarKanse mentioned this pull request Feb 21, 2024
Closed
@IshwarKanse IshwarKanse force-pushed the fix branch 3 times, most recently from d1dbe42 to cc78298 Compare February 21, 2024 12:18
@IshwarKanse
Copy link
Contributor Author

@eddycharly The Github action needs to be updated. I've bumped the version in this PR.

@eddycharly
Copy link
Contributor

@IshwarKanse i just released v0.1.5, will bump the action now :)

@eddycharly
Copy link
Contributor

@IshwarKanse GH action was updated.

@IshwarKanse IshwarKanse force-pushed the fix branch 2 times, most recently from 28c05f9 to 4e2e4be Compare February 21, 2024 13:36
@eddycharly
Copy link
Contributor

Looks like there's another problem now... could be related to multi-cluster support 👀

@IshwarKanse
Copy link
Contributor Author

IshwarKanse commented Feb 21, 2024 •
edited
Loading

For the upgrade one.


    l.go:53: ���������| 13:41:18 | upgrade-test | step-00  | ASSERT    | DONE  | apps/v1/Deployment @ chainsaw-gorgeous-pug/simplest-collector
    l.go:53: ���������| 13:41:18 | upgrade-test | step-00  | TRY       | DONE  |
    l.go:53: ���������| 13:41:18 | upgrade-test | step-01  | TRY       | RUN   |
    l.go:53: ���������| 13:41:18 | upgrade-test | step-01  | SCRIPT    | RUN   |
        === COMMAND
        /usr/bin/sh -c cd ../../../ && make deploy VERSION=e2e
    l.go:53: ���������| 13:41:19 | upgrade-test | step-01  | SCRIPT    | LOG   |
        === STDOUT
        make[1]: Entering directory '/home/runner/work/opentelemetry-operator/opentelemetry-operator'
        /home/runner/work/opentelemetry-operator/opentelemetry-operator/bin/controller-gen "crd:generateEmbeddedObjectMeta=true,maxDescLen=200" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
        cd config/manager && /home/runner/work/opentelemetry-operator/opentelemetry-operator/bin/kustomize edit set image controller=ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:e2e
        /home/runner/work/opentelemetry-operator/opentelemetry-operator/bin/kustomize build config/default | kubectl apply -f -
        make[1]: Leaving directory '/home/runner/work/opentelemetry-operator/opentelemetry-operator'
        === STDERR
        # Warning: 'bases' is deprecated. Please use 'resources' instead. Run 'kustomize edit fix' to update your Kustomization automatically.
        # Warning: 'patchesStrategicMerge' is deprecated. Please use 'patches' instead. Run 'kustomize edit fix' to update your Kustomization automatically.
        # Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically.
        # Warning: 'patchesStrategicMerge' is deprecated. Please use 'patches' instead. Run 'kustomize edit fix' to update your Kustomization automatically.
        error: error validating "STDIN": error validating data: failed to download openapi: Get "http://localhost:8080/openapi/v2?timeout=32s": dial tcp [::1]:8080: connect: connection refused; if you choose to ignore these errors, turn validation off with --validate=false
        make[1]: *** [Makefile:159: deploy] Error 1
    l.go:53: ���������| 13:41:19 | upgrade-test | step-01  | SCRIPT    | ERROR |
        === ERROR
        exit status 2

@eddycharly
Copy link
Contributor

eddycharly commented Feb 21, 2024 •
edited
Loading

I'm cutting v0.1.6 with the fix.

For ref kyverno/chainsaw#946

@eddycharly
Copy link
Contributor

@IshwarKanse v0.1.6 is available.

@IshwarKanse
Copy link
Contributor Author

@eddycharly That was super fast, thanks for fixing these issues. I've updated the PR

@eddycharly
Copy link
Contributor

@IshwarKanse i knew multi cluster support was a risky feature. Thanks for your patience!

@IshwarKanse
Copy link
Contributor Author

@frzifus @jaronoff97 Thanks to @eddycharly ,the Chainsaw bugs have now been resolved and the CI is green. The PR is good to merge.

@jaronoff97 jaronoff97 merged commit 699e034 into open-telemetry:main Feb 21, 2024
29 checks passed
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
@IshwarKanse
Fix tests migrated to Chainsaw (open-telemetry#2650)
c3b4d13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddycharly eddycharly eddycharly left review comments

@iblancasa iblancasa iblancasa approved these changes

@frzifus frzifus frzifus approved these changes

@swiatekm swiatekm swiatekm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@IshwarKanse @frzifus @eddycharly @iblancasa @swiatekm @jaronoff97