Update istio to v1.14.1 #6271

ScheererJ · 2022-07-05T15:26:23Z

How to categorize this PR?

/area networking
/kind enhancement

What this PR does / why we need it:
Update istio to v1.14.1.

Which issue(s) this PR fixes:
None

Special notes for your reviewer:
The istio go library is updated only to v1.14.0 as there is no corresponding release, yet. However, this should be sufficient for now.

Release note:

Update istio to v1.14.1.

/cc @DockToFuture

ScheererJ · 2022-07-05T15:26:47Z

/hold
#6270 needs to be resolved for this to work with exposure classes.

ScheererJ · 2022-07-08T13:12:37Z

/unhold
as #6285 will address #6270.

go.mod

plkokanov

Just a few nits (also in comments that were not part of the PR, but are close to the modified code and could also be improved)
I still haven't gone through all the changes in the chart files though.

plkokanov · 2022-07-12T17:18:38Z

pkg/operation/botanist/component/istio/cmputils.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package istio


Suggested change

package istio

package istio_test

Since these methods appear to only be used in the istio_test package and so that they do not get imported every time the istio package is inported

Actually, they are not used in istio_test at all, but in kubeapiserverexposure_test, vpnauthzserver_test and vpnseedserver_test. This is why I added them simply to istio package to have them in a common place where they fit.
Feel free to suggest a different place, but istio_test does not work as the functions need to be imported in three different locations.

Ah sorry. You could move the BeComparableToMatcher matcher to pkg/utils/test/matchers.
Not sure what would be the best place for the CmpOptsFor... helper functions though.

Any reason why CmpOptsFor... should not be in pkg/utils/test/matchers as well? Those functions only make sense for the BeComparableToMatcher (or if go-cmp would be used separately).

My reasoning would be that BeComparableToMatcher is pretty generic and can be used for other tests as well, whereas the current CmpOptsFor... functions are particularly related to istio.
I dug a bit in our folders and I think you could place the helper functions in e.g. pkg/botanist/component/test/istiocomponent.go

Split the content into pkg/utils/test/matchers and pkg/botanist/component/test/istiocomponent.go as suggested.
Let me know in case further changes are required.

pkg/operation/botanist/component/istio/istiod.go

pkg/operation/botanist/component/vpnseedserver/vpn_seed_server.go

pkg/operation/botanist/component/istio/cmputils.go

plkokanov · 2022-07-13T07:10:12Z

pkg/operation/botanist/component/istio/charts/istio/istio-ingress/templates/deployment.yaml

@@ -35,12 +41,16 @@ spec:
        image: {{ .Values.image }}
        imagePullPolicy: IfNotPresent
        securityContext:
-          allowPrivilegeEscalation: false
+          # Safe since 1.22: https://github.com/kubernetes/kubernetes/pull/103326


Hmm here's the same comment, but no check if k8s version is higher than 1.22. Is that ok?

Indeed in the original (https://github.com/istio/istio/blob/1.14.1/manifests/charts/gateway/templates/deployment.yaml#L52), there is a corresponding check for the kubernetes version surrounding the block. However, we already used this configuration before, which means it already worked. I added the comment to reduce the diff while retaining the same behaviour as before.
Apparently, with kubernetes versions below 1.22 we could not use privileged ports in istio, which was never a loss for us.

ScheererJ · 2022-07-13T09:20:23Z

/retest

ScheererJ · 2022-07-13T11:00:44Z

/retest

DockToFuture

I would also add the app.kubernetes.io/version: "1.14.1" label to the istio-ingress gateway as discussed, apart from that the PR looks good. :)

plkokanov · 2022-07-14T06:52:50Z

The managedseed test seems to fail with:

 <*errors.errorString | 0xc00019d0d0>: {
          s: "seed \"/seed-wp6dg\" is not deleted yet: Failed to delete Seed Cluster (1 error occurred:\n\t* task \"Destroy Istio CRDs\" failed: failed to delete manifests: 1 error occurred: could not delete object of kind \"CustomResourceDefinition\" \"default/proxyconfigs.networking.istio.io\": customresourcedefinitions.apiextensions.k8s.io \"proxyconfigs.networking.istio.io\" is forbidden: User \"system:serviceaccount:garden:gardenlet\" cannot delete resource \"customresourcedefinitions\" in API group \"apiextensions.k8s.io\" at the cluster scope\n\n).",
      }
      seed "/seed-wp6dg" is not deleted yet: Failed to delete Seed Cluster (1 error occurred:
      	* task "Destroy Istio CRDs" failed: failed to delete manifests: 1 error occurred: could not delete object of kind "CustomResourceDefinition" "default/proxyconfigs.networking.istio.io": customresourcedefinitions.apiextensions.k8s.io "proxyconfigs.networking.istio.io" is forbidden: User "system:serviceaccount:garden:gardenlet" cannot delete resource "customresourcedefinitions" in API group "apiextensions.k8s.io" at the cluster scope

"proxyconfigs.networking.istio.io", which was added with the update of the istio crds, should be added to the gardenlet clusterrole:

gardener/charts/gardener/gardenlet/charts/runtime/templates/clusterrole-gardenlet.yaml

Lines 124 to 142 in d829ed0

    
           - apiGroups: 
        
             - apiextensions.k8s.io 
        
             resources: 
        
             - customresourcedefinitions 
        
             resourceNames: 
        
             - hvpas.autoscaling.k8s.io 
        
             - destinationrules.networking.istio.io 
        
             - envoyfilters.networking.istio.io 
        
             - gateways.networking.istio.io 
        
             - serviceentries.networking.istio.io 
        
             - sidecars.networking.istio.io 
        
             - virtualservices.networking.istio.io 
        
             - authorizationpolicies.security.istio.io 
        
             - peerauthentications.security.istio.io 
        
             - requestauthentications.security.istio.io 
        
             - workloadentries.networking.istio.io 
        
             - workloadgroups.networking.istio.io 
        
             - telemetries.telemetry.istio.io 
        
             - wasmplugins.extensions.istio.io

ScheererJ · 2022-07-14T08:16:18Z

"proxyconfigs.networking.istio.io", which was added with the update of the istio crds, should be added to the gardenlet clusterrole:

Thanks for pointing this out. I added it in the latest commit (together with the version labels suggested by @DockToFuture).

ScheererJ · 2022-07-14T08:17:02Z

I would also add the app.kubernetes.io/version: "1.14.1" label to the istio-ingress gateway as discussed, apart from that the PR looks good. :)

Addressed with latest commit.

gardener-prow · 2022-07-14T09:26:20Z

@ScheererJ: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gardener-apidiff	`b378c04`	link	false	`/test pull-gardener-apidiff`

Full PR test history. Your PR dashboard. Command help for this repository.

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. I understand the commands that are listed here.

plkokanov

Thanks!
/lgtm

gardener-prow · 2022-07-14T09:59:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: plkokanov

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:

~~OWNERS~~ [plkokanov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ialidzhikov · 2022-07-14T13:08:04Z

I am curious how you tested this change in local setup guys?

ialidzhikov · 2022-07-14T13:11:40Z

I am curious how you tested this change in local setup guys?

PTAL #6330

ScheererJ · 2022-07-14T13:11:45Z

I am curious how you tested this change in local setup guys?

I tested this in the "old" local setup, meaning I used a shooted seed.

plkokanov · 2022-07-14T13:16:46Z

I ran it in the entirely local setup, newly created seed, newly created shoot. Then I also tried creating a seed before the istio update, then checking out the change with the istio update and restarting gardenlet, to see if updates to already existing seeds would cause errors, but didn't encounter any problems.

* 'master' of github.com:gardener/gardener: (51 commits) Switch extension controller to `logr` and streamline/cleanup logs (gardener#6332) Switch `./test/...` packages to `logr` and drop `github.com/sirupsen/logrus` dependency (gardener#6316) Only check shoot conditions during hibernation integration test (gardener#6325) Add dashboard for monitoring conntrack race failures. (gardener#6329) Reconcile quota before rbac (gardener#6326) Update istio to v1.14.1 (gardener#6271) Update gardenlet's base image to alpine:3.16.0 (gardener#6321) Update envoy proxy to v1.21.4 (gardener#6320) Deploy the metrics server to the kind cluster (gardener#6301) Fix tools download for aarch64 (arm64) (gardener#6314) update with latest CA releases (gardener#6295) Add missing unit tests for the predicates provided by the extensions library (gardener#6249) [GEP-19] Monitoring Stack - Migrating to the `prometheus-operator` (gardener#6151) Revert "Recreate DWD deployment if needed" (gardener#6307) Update to golang 1.18.4 (gardener#6300) Cleaned up imports in vpn-seed-server (gardener#6315) Prepare next Dev Cycle v1.52.0-dev Release v1.51.0 Add pre/post reconciliation/deletion hooks for the Worker resource (gardener#6290) Update the supported values in the usage text of the `--leader-election-resource-lock` flag (gardener#6304) ...

gardener-prow bot requested a review from DockToFuture July 5, 2022 15:26

gardener-prow bot added area/networking Networking related kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 5, 2022

gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 5, 2022

ScheererJ force-pushed the istio/update-1.14.1 branch from 8010428 to 00c9f61 Compare July 8, 2022 13:11

gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 8, 2022

ScheererJ force-pushed the istio/update-1.14.1 branch 3 times, most recently from 56fec0c to de247bf Compare July 11, 2022 06:58

timebertt reviewed Jul 11, 2022

View reviewed changes

go.mod Show resolved Hide resolved

go.mod Show resolved Hide resolved

ScheererJ force-pushed the istio/update-1.14.1 branch 3 times, most recently from 4958e92 to 17f01a5 Compare July 11, 2022 14:47

plkokanov self-assigned this Jul 12, 2022

plkokanov requested changes Jul 12, 2022

View reviewed changes

plkokanov reviewed Jul 13, 2022

View reviewed changes

ScheererJ force-pushed the istio/update-1.14.1 branch from 17f01a5 to d1e35d9 Compare July 13, 2022 07:40

ScheererJ force-pushed the istio/update-1.14.1 branch from d1e35d9 to 2898c1a Compare July 13, 2022 14:59

gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2022

ScheererJ force-pushed the istio/update-1.14.1 branch from 2898c1a to 0937c58 Compare July 13, 2022 15:06

gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2022

DockToFuture reviewed Jul 13, 2022

View reviewed changes

ScheererJ force-pushed the istio/update-1.14.1 branch from 0937c58 to 973439b Compare July 14, 2022 08:14

Update istio to v1.14.1

b378c04

ScheererJ force-pushed the istio/update-1.14.1 branch from 973439b to b378c04 Compare July 14, 2022 09:24

plkokanov approved these changes Jul 14, 2022

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2022

gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2022

gardener-prow bot merged commit 8a6dce8 into gardener:master Jul 14, 2022

acumino pushed a commit to acumino/gardener that referenced this pull request Jul 14, 2022

Update istio to v1.14.1 (gardener#6271)

752dd42

ialidzhikov mentioned this pull request Jul 14, 2022

After #6271 istio-ingressgateway fails to get ready #6330

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update istio to v1.14.1 #6271

Update istio to v1.14.1 #6271

ScheererJ commented Jul 5, 2022

ScheererJ commented Jul 5, 2022

ScheererJ commented Jul 8, 2022

plkokanov left a comment

plkokanov Jul 12, 2022

ScheererJ Jul 13, 2022

plkokanov Jul 13, 2022

ScheererJ Jul 13, 2022

plkokanov Jul 13, 2022 •

edited

ScheererJ Jul 13, 2022

plkokanov Jul 13, 2022

ScheererJ Jul 13, 2022

ScheererJ commented Jul 13, 2022

ScheererJ commented Jul 13, 2022

DockToFuture left a comment

plkokanov commented Jul 14, 2022 •

edited

ScheererJ commented Jul 14, 2022

ScheererJ commented Jul 14, 2022

gardener-prow bot commented Jul 14, 2022 •

edited

plkokanov left a comment •

edited

gardener-prow bot commented Jul 14, 2022

ialidzhikov commented Jul 14, 2022

ialidzhikov commented Jul 14, 2022 •

edited

ScheererJ commented Jul 14, 2022

plkokanov commented Jul 14, 2022 •

edited

Update istio to v1.14.1 #6271

Update istio to v1.14.1 #6271

Conversation

ScheererJ commented Jul 5, 2022

ScheererJ commented Jul 5, 2022

ScheererJ commented Jul 8, 2022

plkokanov left a comment

Choose a reason for hiding this comment

plkokanov Jul 12, 2022

Choose a reason for hiding this comment

ScheererJ Jul 13, 2022

Choose a reason for hiding this comment

plkokanov Jul 13, 2022

Choose a reason for hiding this comment

ScheererJ Jul 13, 2022

Choose a reason for hiding this comment

plkokanov Jul 13, 2022 • edited

Choose a reason for hiding this comment

ScheererJ Jul 13, 2022

Choose a reason for hiding this comment

plkokanov Jul 13, 2022

Choose a reason for hiding this comment

ScheererJ Jul 13, 2022

Choose a reason for hiding this comment

ScheererJ commented Jul 13, 2022

ScheererJ commented Jul 13, 2022

DockToFuture left a comment

Choose a reason for hiding this comment

plkokanov commented Jul 14, 2022 • edited

ScheererJ commented Jul 14, 2022

ScheererJ commented Jul 14, 2022

gardener-prow bot commented Jul 14, 2022 • edited

plkokanov left a comment • edited

Choose a reason for hiding this comment

gardener-prow bot commented Jul 14, 2022

ialidzhikov commented Jul 14, 2022

ialidzhikov commented Jul 14, 2022 • edited

ScheererJ commented Jul 14, 2022

plkokanov commented Jul 14, 2022 • edited

plkokanov Jul 13, 2022 •

edited

plkokanov commented Jul 14, 2022 •

edited

gardener-prow bot commented Jul 14, 2022 •

edited

plkokanov left a comment •

edited

ialidzhikov commented Jul 14, 2022 •

edited

plkokanov commented Jul 14, 2022 •

edited