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

Add ambient profile #1133

Merged
merged 4 commits into from
Jun 21, 2023
Merged

Add ambient profile #1133

merged 4 commits into from
Jun 21, 2023
+21,327 −4

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented Jun 16, 2023

As per title, this patch adds ambient profile under third_party and github action.

Fix #1103

@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 16, 2023
@codecov
Copy link

codecov bot commented Jun 16, 2023 •
edited
Loading

Codecov Report

Merging #1133 (c11ad52) into main (d70c74b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1133   +/-   ##
=======================================
  Coverage   81.29%   81.29%           
=======================================
  Files          18       18           
  Lines        1588     1588           
=======================================
  Hits         1291     1291           
  Misses        213      213           
  Partials       84       84

@nak3 nak3 mentioned this pull request Jun 16, 2023
Merged
@xtreme-vikram-yadav
Copy link

👍 Aside from the shellcheck errors, it looks good.

@nak3
Copy link
Contributor Author

nak3 commented Jun 20, 2023

Thank you! The sell check error is from the code which I haven't touched. It seems the error is surfaced due to the file change.
So I think it is better to fix it by a separate PR.

@nak3 nak3 mentioned this pull request Jun 20, 2023
Merged
@nak3
Copy link
Contributor Author

nak3 commented Jun 20, 2023

/cc @dprotaso @psschwei @ReToCode

Is it alright to add this ambient profile? Or do we need some discussion for this?

@knative-prow knative-prow bot requested review from dprotaso, psschwei and ReToCode June 20, 2023 02:03
@nak3 nak3 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2023
@psschwei
Copy link
Contributor

No issues adding it from my end.

dprotaso
dprotaso reviewed Jun 20, 2023
View reviewed changes
third_party/istio-latest/istio-kind-ambient.yaml
gateways:
istio-ingressgateway:
autoscaleEnabled: false
type: NodePort
Copy link
Contributor

Choose a reason for hiding this comment

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

MetalLB works so well with kind - I don't think we want to use NodePort anymore

https://kind.sigs.k8s.io/docs/user/loadbalancer/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we don't want to use NodePort. But current default one (istio-kind-no-mesh.yaml) also has the NodePort so if you are alright I will change it in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure

@dprotaso
Copy link
Contributor

Is it alright to add this ambient profile?

Sure

@dprotaso
Copy link
Contributor

/lgtm
/approve

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

knative-prow bot commented Jun 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@nak3
Copy link
Contributor Author

nak3 commented Jun 21, 2023

/test latest-mesh

@nak3 nak3 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 21, 2023
@knative-prow knative-prow bot merged commit 5bf11ed into knative-extensions:main Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dprotaso dprotaso

@upodroid upodroid

@xtreme-vikram-yadav xtreme-vikram-yadav

@psschwei psschwei

@ReToCode ReToCode

Assignees

@dprotaso dprotaso

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use ambient mesh instead of current sidecar mesh for CI
4 participants
@nak3 @xtreme-vikram-yadav @psschwei @dprotaso