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

Switching revision in IstioOperator istio-gateway cache fix #42529

Merged

Conversation

chauhanshubham
Copy link
Contributor

@chauhanshubham chauhanshubham commented Dec 20, 2022

Signed-off-by: Shubham Chauhan shubham@tetrate.io

Please provide a description of this PR:
Considering the following scenario

  1. Setup multi-revisioned Istio control plane as mentioned in https://istio.io/latest/docs/setup/upgrade/canary/
    Assuming there are two istio control plane revisions running - rev1 and rev2
  2. Create IstioOperator iop1 with revision rev1 with ingressGateway enabled. This is reconciled by istio-operator-rev1 and skipped by istio-operator-rev2.
  3. Update IstioOperator iop1 to use revision rev2. The related objects like Deployment/ClusterRole/Service etc. must now be reconciled by istio-operator-rev2.
  4. Update IstioOperator iop1 to move back to revision rev1. istio-operator-rev1 does not re-apply any related object manifests.

After Step 3), istio objects like the gateway Deployment etc. are not reconciled by istio-operator-rev1. This is because istio-operator-rev1 never garbage collected objects from its local cache in Step 2) when the revision was updated.

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 20, 2022
@google-cla
Copy link

google-cla bot commented Dec 20, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@istio-policy-bot
Copy link

😊 Welcome @chauhanshubham! This is either your first contribution to the Istio istio repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 20, 2022
@istio-testing
Copy link
Collaborator

Hi @chauhanshubham. 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.

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
@chauhanshubham chauhanshubham marked this pull request as ready for review December 21, 2022 09:47
@chauhanshubham chauhanshubham requested a review from a team as a code owner December 21, 2022 09:47
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 21, 2022
@deva26
Copy link
Contributor

deva26 commented Dec 21, 2022

Thanks @chauhanshubham for this. Good catch.

@shivanshuraj1333
Copy link
Contributor

/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 Dec 23, 2022
@shivanshuraj1333
Copy link
Contributor

/release-note-none

@shivanshuraj1333 shivanshuraj1333 added the release-notes-none Indicates a PR that does not require release notes. label Jan 9, 2023
@shivanshuraj1333
Copy link
Contributor

/retest

@zirain zirain removed the release-notes-none Indicates a PR that does not require release notes. label Jan 10, 2023
@zirain
Copy link
Member

zirain commented Jan 10, 2023

will take a look later, but this worth a release-notes.

@zirain
Copy link
Member

zirain commented Jan 11, 2023

from this doc, I think change revision in IOP was not recommended.

@howardjohn do you recall something about this?

@howardjohn
Copy link
Member

howardjohn commented Jan 11, 2023 via email

Copy link
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

lgtm

@chauhanshubham
Copy link
Contributor Author

/test integ-security-multicluster

@chauhanshubham
Copy link
Contributor Author

@zirain @howardjohn @morvencao do you see a need for release notes for this commit?

@howardjohn howardjohn added the release-notes-none Indicates a PR that does not require release notes. label Jan 20, 2023
@howardjohn
Copy link
Member

/retest

@istio-testing istio-testing merged commit c777f65 into istio:master Jan 20, 2023
bleggett added a commit to bleggett/istio that referenced this pull request Jan 24, 2023
Update protobufs

Add dep

Derp

Consider actually using TCP

Hmm

Fix this

Missed one

Add log message

Logging

Remove these

Testing

Missed this

gawd

Hmm

Fix this

Testing

Ugh

Whitespace

Testing

Invalid component

Testing

Fix logging

Testt

Add alt case

Hmm

Fix

Fix 2

Fix routing

Actually fix

Fix this

Don't cleanup gateways

Aight

I'm a goofus

Tidy

Testing

Test

Hmm

Test

Fix this

Hmmm

Oh

Fix this

Fix this

test

Hmm

Test

Hmm

Hmm

Cleanup

Revert "Cleanup"

This reverts commit 1f933b720c504dc82cdb5683e1e08e6d464f5f1a.

Fix this

Better comment

Fix comment

Automator: update proxy@master in istio/istio@master (istio#42865)

Automator: update common-files@master in istio/istio@master (istio#42879)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42880)

improve wasm ecds comments (istio#42883)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Change no server certificate verify as warning level (istio#42876)

* Change no server certificate verify as warning level

* update

Use latest helm dependency (istio#42891)

Automator: update proxy@master in istio/istio@master (istio#42898)

Automator: update common-files@master in istio/istio@master (istio#42900)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42901)

Automator: update proxy@master in istio/istio@master (istio#42907)

Cleanup unused metadata (istio#42903)

Update BASE_VERSION to master-2023-01-20T04-29-21 (istio#42913)

Add Locality info to debug interface (istio#42884)

Switching revision in IstioOperator istio-gateway cache fix (istio#42529)

* switching revision in IstioOperator istio-gateway cache fix

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

* include all components

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Automator: update proxy@master in istio/istio@master (istio#42923)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42926)

add e2e for MetricDefinition (istio#42563) (istio#42924)

* add e2e for MetricDefinition

* fix lint

* send http traffic only

* fix prom query

Co-authored-by: zirain <hejianpeng2@huawei.com>

tf: ensure Check is always set (istio#42908)

* tf: ensure Check is always set

Avoid cases where we do not test anything

* Always set

* a few more

* fix

Initial messing about for PROXY protocol tests

Fix this

Automator: update proxy@master in istio/istio@master (istio#42865)

Automator: update common-files@master in istio/istio@master (istio#42879)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42880)

improve wasm ecds comments (istio#42883)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Change no server certificate verify as warning level (istio#42876)

* Change no server certificate verify as warning level

* update

Use latest helm dependency (istio#42891)

Automator: update proxy@master in istio/istio@master (istio#42898)

Automator: update common-files@master in istio/istio@master (istio#42900)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42901)

Automator: update proxy@master in istio/istio@master (istio#42907)

Cleanup unused metadata (istio#42903)

Update BASE_VERSION to master-2023-01-20T04-29-21 (istio#42913)

Add Locality info to debug interface (istio#42884)

Switching revision in IstioOperator istio-gateway cache fix (istio#42529)

* switching revision in IstioOperator istio-gateway cache fix

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

* include all components

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Automator: update proxy@master in istio/istio@master (istio#42923)

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42926)

add e2e for MetricDefinition (istio#42563) (istio#42924)

* add e2e for MetricDefinition

* fix lint

* send http traffic only

* fix prom query

Co-authored-by: zirain <hejianpeng2@huawei.com>

tf: ensure Check is always set (istio#42908)

* tf: ensure Check is always set

Avoid cases where we do not test anything

* Always set

* a few more

* fix

wasm: Send nack when there is not well-formed resource messages (istio#42928)

* Send nack when there is an unmarshal error.

Change-Id: I8143ba8bc8c292ce2de5abda0a4f0c70c111bf80

* change if structure to have more readability

Change-Id: I076929d14dd1312b3717fe992352520223997354

* add unit tests

Change-Id: I31831ea1ba87c99ffc0451a7f3a2cb02acc14d91

Automator: update proxy@master in istio/istio@master (istio#42931)

Automator: update go dependencies@ in istio/istio@master (istio#42932)

Fix TestDeltaEDS flake (istio#42925)

improve appsv1 package import statement (istio#42946)

Signed-off-by: xin.li <xin.li@daocloud.io>

Signed-off-by: xin.li <xin.li@daocloud.io>

Use the slices package for contains check (istio#42947)

Automator: update common-files@master in istio/istio@master (istio#42948)

reducing leaseduration, renewdeadline and retryperiod to shorten test time (istio#42920)

add patch name to envoy filter crash log line (istio#42935)

* add patch name to envoy filter crash log line

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

* log line change

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

use authn filter constant every where (istio#42936)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Automator: update istio/client-go@master dependency in istio/istio@master (istio#42949)

Automator: update proxy@master in istio/istio@master (istio#42950)

Automator: update proxy@master in istio/istio@master (istio#42953)

Update master branch from 1.17 to 1.18 (istio#42875)

* Update master branch from 1.17 to 1.18

* Remove older envoy filters

* Fixup some version errors

Update default k8s for test and buildkit (istio#42952)

Use helm repo for integration tests instead of tarballs (istio#42864)

* Use helm repo for integration tests instead of tarballs

Signed-off-by: Faseela K <faseela.k@est.tech>

* refactor

Signed-off-by: Faseela K <faseela.k@est.tech>

* configuration option for helm repo

Signed-off-by: Faseela K <faseela.k@est.tech>

* fix lint failure

Signed-off-by: Faseela K <faseela.k@est.tech>

* add --repo to individual commands

Signed-off-by: Faseela K <faseela.k@est.tech>

Signed-off-by: Faseela K <faseela.k@est.tech>

Automator: update common-files@master in istio/istio@master (istio#42957)
istio-testing pushed a commit that referenced this pull request Jan 31, 2023
* Test

* Add basic integration tests for PROXY protocol and gateway

Update protobufs

Add dep

Derp

Consider actually using TCP

Hmm

Fix this

Missed one

Add log message

Logging

Remove these

Testing

Missed this

gawd

Hmm

Fix this

Testing

Ugh

Whitespace

Testing

Invalid component

Testing

Fix logging

Testt

Add alt case

Hmm

Fix

Fix 2

Fix routing

Actually fix

Fix this

Don't cleanup gateways

Aight

I'm a goofus

Tidy

Testing

Test

Hmm

Test

Fix this

Hmmm

Oh

Fix this

Fix this

test

Hmm

Test

Hmm

Hmm

Cleanup

Revert "Cleanup"

This reverts commit 1f933b720c504dc82cdb5683e1e08e6d464f5f1a.

Fix this

Better comment

Fix comment

Automator: update proxy@master in istio/istio@master (#42865)

Automator: update common-files@master in istio/istio@master (#42879)

Automator: update istio/client-go@master dependency in istio/istio@master (#42880)

improve wasm ecds comments (#42883)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Change no server certificate verify as warning level (#42876)

* Change no server certificate verify as warning level

* update

Use latest helm dependency (#42891)

Automator: update proxy@master in istio/istio@master (#42898)

Automator: update common-files@master in istio/istio@master (#42900)

Automator: update istio/client-go@master dependency in istio/istio@master (#42901)

Automator: update proxy@master in istio/istio@master (#42907)

Cleanup unused metadata (#42903)

Update BASE_VERSION to master-2023-01-20T04-29-21 (#42913)

Add Locality info to debug interface (#42884)

Switching revision in IstioOperator istio-gateway cache fix (#42529)

* switching revision in IstioOperator istio-gateway cache fix

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

* include all components

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Automator: update proxy@master in istio/istio@master (#42923)

Automator: update istio/client-go@master dependency in istio/istio@master (#42926)

add e2e for MetricDefinition (#42563) (#42924)

* add e2e for MetricDefinition

* fix lint

* send http traffic only

* fix prom query

Co-authored-by: zirain <hejianpeng2@huawei.com>

tf: ensure Check is always set (#42908)

* tf: ensure Check is always set

Avoid cases where we do not test anything

* Always set

* a few more

* fix

Initial messing about for PROXY protocol tests

Fix this

Automator: update proxy@master in istio/istio@master (#42865)

Automator: update common-files@master in istio/istio@master (#42879)

Automator: update istio/client-go@master dependency in istio/istio@master (#42880)

improve wasm ecds comments (#42883)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Change no server certificate verify as warning level (#42876)

* Change no server certificate verify as warning level

* update

Use latest helm dependency (#42891)

Automator: update proxy@master in istio/istio@master (#42898)

Automator: update common-files@master in istio/istio@master (#42900)

Automator: update istio/client-go@master dependency in istio/istio@master (#42901)

Automator: update proxy@master in istio/istio@master (#42907)

Cleanup unused metadata (#42903)

Update BASE_VERSION to master-2023-01-20T04-29-21 (#42913)

Add Locality info to debug interface (#42884)

Switching revision in IstioOperator istio-gateway cache fix (#42529)

* switching revision in IstioOperator istio-gateway cache fix

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

* include all components

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

Automator: update proxy@master in istio/istio@master (#42923)

Automator: update istio/client-go@master dependency in istio/istio@master (#42926)

add e2e for MetricDefinition (#42563) (#42924)

* add e2e for MetricDefinition

* fix lint

* send http traffic only

* fix prom query

Co-authored-by: zirain <hejianpeng2@huawei.com>

tf: ensure Check is always set (#42908)

* tf: ensure Check is always set

Avoid cases where we do not test anything

* Always set

* a few more

* fix

wasm: Send nack when there is not well-formed resource messages (#42928)

* Send nack when there is an unmarshal error.

Change-Id: I8143ba8bc8c292ce2de5abda0a4f0c70c111bf80

* change if structure to have more readability

Change-Id: I076929d14dd1312b3717fe992352520223997354

* add unit tests

Change-Id: I31831ea1ba87c99ffc0451a7f3a2cb02acc14d91

Automator: update proxy@master in istio/istio@master (#42931)

Automator: update go dependencies@ in istio/istio@master (#42932)

Fix TestDeltaEDS flake (#42925)

improve appsv1 package import statement (#42946)

Signed-off-by: xin.li <xin.li@daocloud.io>

Signed-off-by: xin.li <xin.li@daocloud.io>

Use the slices package for contains check (#42947)

Automator: update common-files@master in istio/istio@master (#42948)

reducing leaseduration, renewdeadline and retryperiod to shorten test time (#42920)

add patch name to envoy filter crash log line (#42935)

* add patch name to envoy filter crash log line

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

* log line change

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

use authn filter constant every where (#42936)

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>

Automator: update istio/client-go@master dependency in istio/istio@master (#42949)

Automator: update proxy@master in istio/istio@master (#42950)

Automator: update proxy@master in istio/istio@master (#42953)

Update master branch from 1.17 to 1.18 (#42875)

* Update master branch from 1.17 to 1.18

* Remove older envoy filters

* Fixup some version errors

Update default k8s for test and buildkit (#42952)

Use helm repo for integration tests instead of tarballs (#42864)

* Use helm repo for integration tests instead of tarballs

Signed-off-by: Faseela K <faseela.k@est.tech>

* refactor

Signed-off-by: Faseela K <faseela.k@est.tech>

* configuration option for helm repo

Signed-off-by: Faseela K <faseela.k@est.tech>

* fix lint failure

Signed-off-by: Faseela K <faseela.k@est.tech>

* add --repo to individual commands

Signed-off-by: Faseela K <faseela.k@est.tech>

Signed-off-by: Faseela K <faseela.k@est.tech>

Automator: update common-files@master in istio/istio@master (#42957)

* Fix these

* Lint fixes

* Fix comments

* Lint fixes

* More lint fixes

* Be slightly more efficient with gateway creation

* Lints

* Update tests/integration/pilot/common/routing.go

Co-authored-by: Lin Sun <lin.sun@solo.io>

---------

Co-authored-by: Lin Sun <lin.sun@solo.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/environments 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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants