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

Test istio with system-internal-tls enabled #14494

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

ReToCode
Copy link
Member

@ReToCode ReToCode commented Oct 10, 2023

Fixes knative-extensions/net-istio#1063

Changes

  • Adds istio with system-internal-tls enabled to test matrix
  • Runs the system-internal-tls tests for istio

@knative-prow knative-prow bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 10, 2023
@knative-prow knative-prow bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1940e5a) 86.01% compared to head (f8f6af4) 86.01%.

❗ Current head f8f6af4 differs from pull request most recent head 0a5a698. Consider uploading reports for the commit 0a5a698 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14494   +/-   ##
=======================================
  Coverage   86.01%   86.01%           
=======================================
  Files         197      197           
  Lines       14915    14915           
=======================================
  Hits        12829    12829           
  Misses       1776     1776           
  Partials      310      310           

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

@nak3
Copy link
Contributor

nak3 commented Oct 11, 2023

Could you also try to enable this

if !(strings.Contains(test.ServingFlags.IngressClass, "kourier") || strings.Contains(test.ServingFlags.IngressClass, "contour")) {
t.Skip("Skip this test for non-kourier or non-contour ingress.")
}
?

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 11, 2023
@ReToCode
Copy link
Member Author

Could you also try to enable this

Done, let's see if that works. I also changed the if to avoid having all not-cases in there.

@ReToCode
Copy link
Member Author

/test istio-latest-no-mesh

@ReToCode
Copy link
Member Author

@nak3 looks good with the test enabled. I'll update this PR here to be able to merge once the net-istio PR is merged.

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2023
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2023
@ReToCode ReToCode changed the title [wip] Test istio with system-internal-tls enabled Test istio with system-internal-tls enabled Oct 17, 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 Oct 17, 2023
@ReToCode
Copy link
Member Author

/hold for knative-extensions/net-istio#1085 to be merged and bumped in Serving

@nak3
Copy link
Contributor

nak3 commented Oct 17, 2023

/lgtm
/approve

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

knative-prow bot commented Oct 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3, ReToCode

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

@ReToCode
Copy link
Member Author

/retest

1 similar comment
@ReToCode
Copy link
Member Author

/retest

@ReToCode
Copy link
Member Author

/unhold

@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 Oct 18, 2023
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2023
@ReToCode
Copy link
Member Author

Rebased to have net-istio changes in, @nak3 can you approve again?

@nak3
Copy link
Contributor

nak3 commented Oct 18, 2023

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2023
@knative-prow knative-prow bot merged commit 707d286 into knative:main Oct 18, 2023
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features 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
Development

Successfully merging this pull request may close these issues.

Support system-internal-tls in net-istio
3 participants