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

Use correct cluster domain in e2e tests #7140

Merged
merged 4 commits into from
Aug 3, 2023
Merged

Use correct cluster domain in e2e tests #7140

merged 4 commits into from
Aug 3, 2023

Conversation

creydr
Copy link
Member

@creydr creydr commented Aug 2, 2023 •
edited
Loading

Currently some e2e tests running on kind are failing due to errors like:

expecting extension 'knativeerrordest' equal to 'http://failer-witdata-nprfdmbb.test-bepfnesy.svc.c5728157394.local', got 'http://failer-witdata-nprfdmbb.test-bepfnesy.svc.cluster.local'

This is, because some tests assume a cluster domain of cluster.local, while we set them for the e2e tests on kind:

eventing/.github/workflows/kind-e2e.yaml

Lines 115 to 120 in 8a09756

- name: Run e2e Tests
run: |
export CLUSTER_DOMAIN=${CLUSTER_SUFFIX}
# Run the tests tagged as e2e on the KinD cluster.
go test -race -count=1 -parallel=12 -timeout=50m -tags=e2e \
${{ matrix.test-suite }} ${{ matrix.extra-test-flags }}

eventing/.github/workflows/kind-e2e.yaml

Lines 46 to 55 in 8a09756

env:
KO_DOCKER_REPO: kind.local
SYSTEM_NAMESPACE: knative-eventing
# Use a semi-random cluster suffix, but somewhat predictable
# so reruns don't just give us a completely new value.
CLUSTER_SUFFIX: c${{ github.run_id }}.local
ARTIFACTS: ${{ github.workspace }}/artifacts
NODE_VERSION: ${{ matrix.k8s-version }}
NODE_SHA: ${{ matrix.kind-image-sha }}

This PR addresses it and gets the cluster domain from the CLUSTER_DOMAIN env.

/assign @pierDipi @matzew

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release labels Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: +0.01% 🎉

Comparison is base (8a09756) 77.93% compared to head (622bc11) 77.94%.
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7140      +/-   ##
==========================================
+ Coverage   77.93%   77.94%   +0.01%     
==========================================
  Files         248      248              
  Lines       13332    13257      -75     
==========================================
- Hits        10390    10333      -57     
+ Misses       2408     2398      -10     
+ Partials      534      526       -8

see 19 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@creydr
Copy link
Member Author

creydr commented Aug 2, 2023

/hold
need testing

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2023
@creydr creydr changed the title Use correct cluster domain in e2e tests [WIP] Use correct cluster domain in e2e tests Aug 2, 2023
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 2, 2023
@pierDipi
Copy link
Member

pierDipi commented Aug 2, 2023

Supporting this kind of testing is more involved, especially with TLS where certificates have specific DNS names

pierDipi
pierDipi reviewed Aug 2, 2023
View reviewed changes
test/utils/utils.go Outdated Show resolved Hide resolved
@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 2, 2023
@creydr
Copy link
Member Author

creydr commented Aug 2, 2023

Just as FYI: Seems we have the kind e2e/rekt failures since the k8s version bump to 1.25: 2899fce (56e9ee1 merged before and the tests still passed). From commit history: https://github.com/knative/eventing/commits/main?after=3915df7f856089af51860c43a73a12f8462c0ceb+259&branch=main&qualified_name=refs%2Fheads%2Fmain

@creydr
Propagate cluster suffix to kubeadm
622bc11 
Acccording to
kubernetes-sigs/kind#1466 (comment)
it is not needed anymore to specify the apiVersion of the
ClusterConfiguration. This also fixes that the config is propagated to
kubeadm correctly.
@creydr creydr force-pushed the use-correct-clusterdomain-in-e2e-tests branch from 07a9fbc to 622bc11 Compare August 3, 2023 07:11
@creydr creydr mentioned this pull request Aug 3, 2023
Merged
@creydr creydr changed the title [WIP] Use correct cluster domain in e2e tests Use correct cluster domain in e2e tests Aug 3, 2023
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2023
@creydr
Copy link
Member Author

creydr commented Aug 3, 2023

/unhold
/retest

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2023
@creydr
Copy link
Member Author

creydr commented Aug 3, 2023

/test upgrade-tests

1 similar comment
@creydr
Copy link
Member Author

creydr commented Aug 3, 2023

/test upgrade-tests

@creydr
Copy link
Member Author

creydr commented Aug 3, 2023

/assign @pierDipi

pierDipi
pierDipi approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2023
@knative-prow
Copy link

knative-prow bot commented Aug 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, pierDipi

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

@knative-prow knative-prow bot merged commit bfba6fd into knative:main Aug 3, 2023
24 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierDipi pierDipi pierDipi approved these changes

@aslom aslom Awaiting requested review from aslom

@evankanderson evankanderson Awaiting requested review from evankanderson

Assignees

@matzew matzew

@pierDipi pierDipi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@creydr @pierDipi @matzew