Skip to content

Merge changes from istio template (#38837) #40410

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
Merged
merged 1 commit into from
Aug 12, 2022
Merged

Merge changes from istio template (#38837) #40410

merged 1 commit into from
Aug 12, 2022

Conversation

sanjaypujare
Copy link
Contributor

@sanjaypujare sanjaypujare commented Aug 11, 2022 •
edited by istio-policy-bot
Loading

  • Merge changes from istio template

  • Remove security section

  • Missing end

  • Make gen, remove the ifs and add lifecycle - grpc starts very fast

  • Few debug statements to figure out why wait doesn't work

  • Use the agent ready port

  • Rever the deugging changes

  • make gen

Please provide a description of this PR:

@costinm @sanjaypujare
Merge changes from istio template (istio#38837)
f8e7fc1 
* Merge changes from istio template

* Remove security section

* Missing end

* Make gen, remove the ifs and add lifecycle - grpc starts very fast

* Few debug statements to figure out why wait doesn't work

* Use the agent ready port

* Rever the deugging changes

* make gen
@istio-testing istio-testing added do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test labels Aug 11, 2022
@istio-testing
Copy link
Collaborator

Hi @sanjaypujare. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@sanjaypujare
Copy link
Contributor Author

@costinm as discussed. Once you are okay I will change from draft to regular PR

@sanjaypujare sanjaypujare mentioned this pull request Aug 11, 2022
Merged
costinm
costinm approved these changes Aug 11, 2022
View reviewed changes
Copy link
Contributor

@costinm costinm left a comment

Choose a reason for hiding this comment

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

This is a safe change to merge - it only impacts proxyless gRPC, which is still an experimental feature.

@costinm
Copy link
Contributor

costinm commented Aug 11, 2022

Will need to be approved by the release managers. I think it is very good to merge since it helps with increasing the number of users of proxyless gRPC, in 1.14 it doesn't work very well without this change.

@sanjaypujare sanjaypujare marked this pull request as ready for review August 11, 2022 18:34
@sanjaypujare sanjaypujare requested a review from a team August 11, 2022 18:34
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Aug 11, 2022
@sanjaypujare
Copy link
Contributor Author

This is a safe change to merge - it only impacts proxyless gRPC, which is still an experimental feature.

Thanks @costinm . I believe I can't merge this but will need to be merged by the release manager?

@sanjaypujare
Copy link
Contributor Author

The tests are still waiting, how does that get unblocked? @lei-tang ?

release-notes_istio_release-1.14 Expected — Waiting for status to be reported        Required

@lei-tang lei-tang added the release-notes-none Indicates a PR that does not require release notes. label Aug 11, 2022
@lei-tang
Copy link
Contributor

The tests are still waiting, how does that get unblocked? @lei-tang ?

release-notes_istio_release-1.14 Expected — Waiting for status to be reported        Required

Similar to #38837, add a release-notes-none label to this PR to unblock the test release-notes_istio_release-1.14.

@lei-tang
Copy link
Contributor

/retest

@sanjaypujare
Copy link
Contributor Author

...
Similar to #38837, add a release-notes-none label to this PR to unblock the test release-notes_istio_release-1.14.

Thank you.

@sanjaypujare
Copy link
Contributor Author

/retest

@istio-testing
Copy link
Collaborator

@sanjaypujare: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

@ericvn
Copy link
Contributor

ericvn commented Aug 12, 2022

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Aug 12, 2022
@sanjaypujare
Copy link
Contributor Author

From the output:

=== RUN   TestWebhook
2022-08-12T16:47:04.217830Z	info	tf	=== BEGIN: Test: 'pilot[TestWebhook]' ===
    webhook_test.go:85: Config rejected: false, expected config rejected: true
2022-08-12T16:47:04.304342Z	info	tf	=== DONE (failed):  Test: 'pilot[TestWebhook] (86.505776ms)' ===
2022-08-12T16:47:04.304453Z	error	tf	=== Dumping Namespace echo-2-55861 State...
2022-08-12T16:47:04.304484Z	error	tf	=== Dumping Istio Deployment State...
2022-08-12T16:47:04.304509Z	error	tf	=== Dumping Namespace external-1-139 State...
2022-08-12T16:47:07.255751Z	info	tf	istioctl ([x internal-debug --all configz]): completed after 0.0340s
2022-08-12T16:47:07.286943Z	info	tf	istioctl ([x internal-debug --all mcsz]): completed after 0.0296s
2022-08-12T16:47:07.329873Z	info	tf	istioctl ([x internal-debug --all clusterz]): completed after 0.0427s
--- FAIL: TestWebhook (9.30s)
=== CONT  TestValidation
...
FAIL
2022-08-12T16:47:15.919272Z	info	tf	=== FAILED: Test Run: 'pilot' (exitCode: 1) ===
...
FAIL	istio.io/istio/tests/integration/pilot	290.402s
...
NoCleanup:         false
...
FAIL
...

This does not seem related to the change in the PR. @costinm or @lei-tang could you confirm?

@sanjaypujare
Copy link
Contributor Author

/retest

@istio-testing istio-testing merged commit e381ee4 into istio:release-1.14 Aug 12, 2022
@sanjaypujare sanjaypujare deleted the release-1.14-backport-38837 branch August 12, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@costinm costinm costinm approved these changes

@lei-tang lei-tang lei-tang approved these changes

Assignees
No one assigned
Labels
area/user experience ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. release-notes-none Indicates a PR that does not require release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sanjaypujare @istio-testing @costinm @lei-tang @ericvn @istio-policy-bot