Updates Gateway Status Checking #12955

danehans · 2023-03-27T16:05:29Z

Please provide a description for what this PR is for.

Updates the Gateway status condition check for the ambient getting started guide due to istio/istio#43975.

And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.

istio-testing · 2023-03-27T16:05:41Z

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

danehans · 2023-03-27T16:06:04Z

xref: #12940

ericvn · 2023-03-27T16:07:08Z

/ok-to-test

But you will need to run the go get and go mod tidy and submit the go.* changes here.

ericvn · 2023-03-27T16:42:19Z

@danehans To fix the gencheck failure, run make clean since you ran some tests locally, and run go mod tidy again.

danehans · 2023-03-27T17:02:47Z

Looking into why TestDocs/tasks/traffic-management/ingress/ingress-sni-passthrough/gtwapi_test.sh is failing.

danehans · 2023-03-27T17:08:29Z

Job doc.test.profile_minimal_istio.io is failing due to #12940 (comment).

ericvn · 2023-03-27T17:19:36Z

@howardjohn mentioned in the CI PR that failing to get the host address may be a bug.

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans · 2023-03-27T19:02:06Z

For the doc.test.profile_minimal_istio.io failing test, I have repro'd the issue. The Gateway's service does not get an IP assigned. Since the _wait_for_gateway func only waits for the Programmed=true" condition, this check passes and the INGRESS_HOST` env var is unassigned.

 build-tools:/work$ kubectl get gtw/mygateway -o yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"gateway.networking.k8s.io/v1beta1","kind":"Gateway","metadata":{"annotations":{},"name":"mygateway","namespace":"default"},"spec":{"gatewayClassName":"istio","listeners":[{"allowedRoutes":{"namespaces":{"from":"All"}},"hostname":"nginx.example.com","name":"https","port":443,"protocol":"TLS","tls":{"mode":"Passthrough"}}]}}
  creationTimestamp: "2023-03-27T18:53:38Z"
  generation: 1
  name: mygateway
  namespace: default
  resourceVersion: "2638"
  uid: 7bed5789-bb15-4931-8c54-c0318ffbdb73
spec:
  gatewayClassName: istio
  listeners:
  - allowedRoutes:
      namespaces:
        from: All
    hostname: nginx.example.com
    name: https
    port: 443
    protocol: TLS
    tls:
      mode: Passthrough
status:
  conditions:
  - lastTransitionTime: "2023-03-27T18:53:38Z"
    message: 'Assigned to service(s) mygateway-istio.default.svc.cluster.local:443,
      but failed to assign to all requested addresses: address pending for hostname
      "mygateway-istio.default.svc.cluster.local"'
    observedGeneration: 1
    reason: NoResources
    status: "False"
    type: Accepted
  - lastTransitionTime: "2023-03-27T18:53:38Z"
    message: Resource programmed
    observedGeneration: 1
    reason: Programmed
    status: "True"
    type: Programmed
...

build-tools:/work$ kubectl get svc
NAME              TYPE           CLUSTER-IP     EXTERNAL-IP   PORT(S)                         AGE
kubernetes        ClusterIP      10.96.0.1      <none>        443/TCP                         19m
my-nginx          ClusterIP      10.96.45.85    <none>        443/TCP                         2m32s
mygateway-istio   LoadBalancer   10.96.68.111   <pending>     15021:31564/TCP,443:31096/TCP   2m28s

danehans · 2023-03-27T20:35:49Z

content/en/docs/ops/ambient/getting-started/index.md

@@ -241,22 +241,22 @@ waypoint default/bookinfo-productpage applied
 View the `productpage` waypoint proxy status; you should see the details of the gateway
 resource with `Ready` status:

-{{< text bash >}}
+{{< text syntax=bash snip_id=none >}}


No longer needed since test is now using Test is now using _wait_for_gateway.

Since the primary purpose of doc tests (with side benefit of testing Istio) is to test that the documented instructions work as documented, a general best practice is to call snips wherever we can. The util functions are intended for docs that don't include explicit instructions, e.g., docs that just say before-you-begin install bookinfo, or something like that.

frankbu · 2023-03-27T20:48:09Z

content/en/docs/tasks/traffic-management/ingress/ingress-sni-passthrough/index.md

+Wait for the Gateway to be accepted:
+
+{{< text syntax=bash snip_id=none >}}
+$ kubectl wait --for=condition=accepted gtw mygateway


The correct semantics of gateway accepted vs programmed vs ready is still not agreed in the upstream gateway-api project, so I think we should wait for resolution before updating any docs.
kubernetes-sigs/gateway-api#1877

@frankbu PTAL at #12955 (comment). The way that Gateway status is currently implemented, Programmed=True can be surfaced when an address is unassigned. This is causing tests that set INGRESS_HOST to fail. As you mention, kubernetes-sigs/gateway-api#1877 is attempting to resolve ambiguity in the conditions. Maybe the best path forward is to check for the presence of both status conditions until this is resolved?

@danehans The problem is that there are 14 other docs that are broken by the current implementation. Fixing the implementation (to at least align with the best guess of where the spec is going) seems like a better approach than hacking all the docs temporarily. (Btw, the fact that the current impl has accepted not true until the address is assigned is an obvious bug since according to the spec accepted just means the config is syntactically valid.)

Xunzhuo · 2023-03-28T10:57:28Z

/retest

ericvn · 2023-03-28T14:12:57Z

Noting also that @howardjohn mentioned in Slack that the underlying change may be reverted so we may need to revert some of our changes as well. Biggest unknown is when and that drives when we can actually update the version of Istio we are testing against.

danehans · 2023-03-28T16:32:19Z

xref: istio/istio#44139 to remove wait conditions for Gateway status.

danehans · 2023-03-28T21:21:35Z

content/en/docs/ops/ambient/getting-started/snips.sh

@@ -119,18 +119,6 @@ snip_l7_authorization_policy_3() {
 kubectl get gtw bookinfo-productpage -o yaml
 }

-! read -r -d '' snip_l7_authorization_policy_4 <<\ENDSNIP


Note: This snip is not currently being used by the test.

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans · 2023-03-28T21:52:15Z

content/en/docs/ops/ambient/getting-started/index.md

@@ -247,16 +247,16 @@ $ kubectl get gtw bookinfo-productpage -o yaml

 Verify that the waypoint proxy status is ready:

-{{< text plaintext >}}
+{{< text syntax=plaintext snip_id=none >}}


Note: This snip is not currently being used by the test.

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

Xunzhuo

LGTM

frankbu · 2023-03-29T12:50:51Z

/test doc.test.profile_none

* Updates Gateway Status Check for Ambient Guide Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io> * Updates status condition checking Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io> * Bumps istio dep Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io> * Generated changes from gen-check Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io> --------- Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans requested a review from a team as a code owner March 27, 2023 16:05

istio-policy-bot added area/ambient kind/docs labels Mar 27, 2023

istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test labels Mar 27, 2023

danehans mentioned this pull request Mar 27, 2023

Adds K8s Gateway API to Getting Started Guide #12932

Merged

10 tasks

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 Mar 27, 2023

danehans requested a review from a team as a code owner March 27, 2023 16:09

danehans force-pushed the waypoint_status_fix branch from 0e4eadd to 4c14eba Compare March 27, 2023 16:44

Updates Gateway Status Check for Ambient Guide

ed2630d

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans force-pushed the waypoint_status_fix branch from 4c14eba to 9931258 Compare March 27, 2023 20:32

danehans requested a review from a team as a code owner March 27, 2023 20:32

danehans commented Mar 27, 2023

View reviewed changes

frankbu reviewed Mar 27, 2023

View reviewed changes

danehans requested a review from a team as a code owner March 27, 2023 21:04

danehans force-pushed the waypoint_status_fix branch 2 times, most recently from f53a2ad to 924c591 Compare March 27, 2023 23:39

danehans force-pushed the waypoint_status_fix branch 2 times, most recently from a86880d to 66fb20e Compare March 28, 2023 14:00

danehans force-pushed the waypoint_status_fix branch 2 times, most recently from a89a244 to f18a5c9 Compare March 28, 2023 15:55

istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 28, 2023

danehans force-pushed the waypoint_status_fix branch from f18a5c9 to 79a6547 Compare March 28, 2023 21:19

istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 28, 2023

danehans commented Mar 28, 2023

View reviewed changes

ericvn mentioned this pull request Mar 28, 2023

Update the test reference and skywalking test #12962

Merged

danehans added 2 commits March 28, 2023 14:43

Updates status condition checking

14881d7

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

Bumps istio dep

236afca

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans force-pushed the waypoint_status_fix branch from 79a6547 to 236afca Compare March 28, 2023 21:51

danehans commented Mar 28, 2023

View reviewed changes

Generated changes from gen-check

3bfd59e

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>

danehans requested review from frankbu and Xunzhuo and removed request for frankbu March 29, 2023 02:37

Xunzhuo approved these changes Mar 29, 2023

View reviewed changes

ericvn approved these changes Mar 29, 2023

View reviewed changes

ericvn mentioned this pull request Mar 29, 2023

Automator: update istio@ test reference #12940

Closed

frankbu approved these changes Mar 29, 2023

View reviewed changes

istio-testing merged commit 0544065 into istio:master Mar 29, 2023
12 checks passed

danehans deleted the waypoint_status_fix branch March 29, 2023 13:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates Gateway Status Checking #12955

Updates Gateway Status Checking #12955

danehans commented Mar 27, 2023

istio-testing commented Mar 27, 2023

danehans commented Mar 27, 2023

ericvn commented Mar 27, 2023

ericvn commented Mar 27, 2023

danehans commented Mar 27, 2023

danehans commented Mar 27, 2023

ericvn commented Mar 27, 2023

danehans commented Mar 27, 2023

danehans Mar 27, 2023

frankbu Mar 27, 2023

frankbu Mar 27, 2023

danehans Mar 27, 2023 •

edited

frankbu Mar 28, 2023

Xunzhuo commented Mar 28, 2023

ericvn commented Mar 28, 2023

danehans commented Mar 28, 2023

danehans Mar 28, 2023

danehans Mar 28, 2023

Xunzhuo left a comment

frankbu commented Mar 29, 2023

Updates Gateway Status Checking #12955

Updates Gateway Status Checking #12955

Conversation

danehans commented Mar 27, 2023

istio-testing commented Mar 27, 2023

danehans commented Mar 27, 2023

ericvn commented Mar 27, 2023

ericvn commented Mar 27, 2023

danehans commented Mar 27, 2023

danehans commented Mar 27, 2023

ericvn commented Mar 27, 2023

danehans commented Mar 27, 2023

danehans Mar 27, 2023

Choose a reason for hiding this comment

frankbu Mar 27, 2023

Choose a reason for hiding this comment

frankbu Mar 27, 2023

Choose a reason for hiding this comment

danehans Mar 27, 2023 • edited

Choose a reason for hiding this comment

frankbu Mar 28, 2023

Choose a reason for hiding this comment

Xunzhuo commented Mar 28, 2023

ericvn commented Mar 28, 2023

danehans commented Mar 28, 2023

danehans Mar 28, 2023

Choose a reason for hiding this comment

danehans Mar 28, 2023

Choose a reason for hiding this comment

Xunzhuo left a comment

Choose a reason for hiding this comment

frankbu commented Mar 29, 2023

danehans Mar 27, 2023 •

edited