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

Create IstioRevisionTag documentation #511

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dgn
Copy link
Collaborator

@dgn dgn commented Dec 6, 2024

This adds a section to the README.md about the IstioRevisionTag API and updates the SEP. I also fixed some typos and minor problems in the existing docs.

Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.30%. Comparing base (02d1c8a) to head (40da26f).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #511   +/-   ##
=======================================
  Coverage   78.30%   78.30%           
=======================================
  Files          38       38           
  Lines        2272     2272           
=======================================
  Hits         1779     1779           
  Misses        403      403           
  Partials       90       90           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@istio-testing
Copy link
Collaborator

PR needs rebase.

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-sigs/prow repository.

name: prod # the name of the Istio/IstioRevision resource
```

As you can see in the YAML above, `IstioRevisionTag` really only has one field in its spec: `targetRef`. With this field, you can reference an `Istio` or `IstioRevision` resource. So after deploying this, you will be able to use both the `istio.io/rev=default` and also `istio-injection=enabled` labels to inject proxies into your workloads. The `istio-injection` label can only be used for revisions named `default` - and this tag is called `default`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"for revisions named default - and this tag is called default"
It might be a little bit confusing.
Maybe something like "can only be used for revisions named default or when you create a IstioRevisionTag named default."

#### Conditions
All resources have a `Ready` condition which is set to `true` as soon as all child resource have been created and are deemed Ready by their respective controllers. To see additional conditions for each of the resources, check the [API reference documentation](https://github.com/istio-ecosystem/sail-operator/tree/main/docs/api-reference/sailoperator.io.md).

#### InUse Detection
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add a few sentences when this is useful from user point of view?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, ok, it's used in the example below. So maybe just one sentence: "See the example bellow for proper usage."

@@ -413,11 +449,11 @@ Steps:
10. Verify the `Istio` and `IstioRevision` resources. There will be a new revision created with the new version.

```console
$ kubectl get istio -n istio-system
$ kubectl get istio
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why removing the namespace here?

NAME REVISIONS READY IN USE ACTIVE REVISION STATUS VERSION AGE
default 2 2 1 default-v1-23-2 Healthy v1.23.2 9m23s

$ kubectl get istiorevision -n istio-system
$ kubectl get istiorevision
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why removing the namespace here?

updateStrategy:
type: RevisionBased
inactiveRevisionDeletionGracePeriodSeconds: 30
version: v1.22.5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
version: v1.22.5
version: v1.23.3

```console
$ kubectl get istio
NAME REVISIONS READY IN USE ACTIVE REVISION STATUS VERSION AGE
default 1 1 1 default-v1-22-5 Healthy v1.22.5 52s
Copy link
Collaborator

@FilipB FilipB Dec 17, 2024

Choose a reason for hiding this comment

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

Suggested change
default 1 1 1 default-v1-22-5 Healthy v1.22.5 52s
default 1 1 1 default-v1-23-3 Healthy v1.23.3 52s

```console
$ kubectl get istiorevisiontags
NAME STATUS IN USE REVISION AGE
default NotReferencedByAnything False default-v1-22-5 52s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default NotReferencedByAnything False default-v1-22-5 52s
default NotReferencedByAnything False default-v1-23-3 52s

```console
$ kubectl get istiorevisiontag
NAME STATUS IN USE REVISION AGE
default Healthy True default-v1-22-5 2m46s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default Healthy True default-v1-22-5 2m46s
default Healthy True default-v1-23-3 2m46s

9. Update the control plane to a new version.

```bash
kubectl patch istio default -n istio-system --type='merge' -p '{"spec":{"version":"v1.23.2"}}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
kubectl patch istio default -n istio-system --type='merge' -p '{"spec":{"version":"v1.23.2"}}'
kubectl patch istio default -n istio-system --type='merge' -p '{"spec":{"version":"v1.24.1"}}'

```console
$ kubectl get istio
NAME REVISIONS READY IN USE ACTIVE REVISION STATUS VERSION AGE
default 2 2 1 default-v1-23-2 Healthy v1.23.2 9m23s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default 2 2 1 default-v1-23-2 Healthy v1.23.2 9m23s
default 2 2 1 default-v1-24-1 Healthy v1.24.1 9m23s


$ kubectl get istiorevision
NAME TYPE READY STATUS IN USE VERSION AGE
default-v1-22-5 Local True Healthy True v1.22.5 10m
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default-v1-22-5 Local True Healthy True v1.22.5 10m
default-v1-23-3 Local True Healthy True v1.23.3 10m

$ kubectl get istiorevision
NAME TYPE READY STATUS IN USE VERSION AGE
default-v1-22-5 Local True Healthy True v1.22.5 10m
default-v1-23-2 Local True Healthy True v1.23.2 66s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default-v1-23-2 Local True Healthy True v1.23.2 66s
default-v1-24-1 Local True Healthy True v1.24.1 66s


$ kubectl get istiorevisiontag
NAME STATUS IN USE REVISION AGE
default Healthy True default-v1-23-2 10m44s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default Healthy True default-v1-23-2 10m44s
default Healthy True default-v1-24-1 10m44s

NAME STATUS IN USE REVISION AGE
default Healthy True default-v1-23-2 10m44s
```
Now, both our IstioRevisions and the IstioRevisionTag are considered in use. The old revision default-v1-22-5 because it is being used by proxies, the new revision default-v1-23-2 because it is referenced by the tag, and lastly the tag because it is referenced by the bookinfo namespace.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Now, both our IstioRevisions and the IstioRevisionTag are considered in use. The old revision default-v1-22-5 because it is being used by proxies, the new revision default-v1-23-2 because it is referenced by the tag, and lastly the tag because it is referenced by the bookinfo namespace.
Now, both our IstioRevisions and the IstioRevisionTag are considered in use. The old revision default-v1-23-3 because it is being used by proxies, the new revision default-v1-24-1 because it is referenced by the tag, and lastly the tag because it is referenced by the bookinfo namespace.

```console
$ kubectl get pods -n istio-system
NAME READY STATUS RESTARTS AGE
istiod-default-v1-22-5-c98fd9675-r7bfw 1/1 Running 0 10m
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
istiod-default-v1-22-5-c98fd9675-r7bfw 1/1 Running 0 10m
istiod-default-v1-23-3-c98fd9675-r7bfw 1/1 Running 0 10m

$ kubectl get pods -n istio-system
NAME READY STATUS RESTARTS AGE
istiod-default-v1-22-5-c98fd9675-r7bfw 1/1 Running 0 10m
istiod-default-v1-23-2-7495cdc7bf-v8t4g 1/1 Running 0 113s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
istiod-default-v1-23-2-7495cdc7bf-v8t4g 1/1 Running 0 113s
istiod-default-v1-24-1-7495cdc7bf-v8t4g 1/1 Running 0 113s

6. Deploy bookinfo application.

```bash
kubectl apply -n bookinfo -f https://raw.githubusercontent.com/istio/istio/release-1.22/samples/bookinfo/platform/kube/bookinfo.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
kubectl apply -n bookinfo -f https://raw.githubusercontent.com/istio/istio/release-1.22/samples/bookinfo/platform/kube/bookinfo.yaml
kubectl apply -n bookinfo -f https://raw.githubusercontent.com/istio/istio/release-1.23/samples/bookinfo/platform/kube/bookinfo.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants