Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2035 +/- ##
==========================================
+ Coverage 76.57% 76.81% +0.23%
==========================================
Files 58 58
Lines 3181 3222 +41
==========================================
+ Hits 2436 2475 +39
- Misses 608 609 +1
- Partials 137 138 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
FilipB
left a comment
There was a problem hiding this comment.
This is great, it would be nice to have some automation which would keep this up to date.
| sail-operator/ | ||
| ├── api/ # CRD type definitions | ||
| │ ├── v1/ # Stable API group (Istio, IstioRevision, IstioRevisionTag, IstioCNI) | ||
| │ └── v1alpha1/ # Experimental API group (ZTunnel) |
There was a problem hiding this comment.
Ztunnel is already in v1 too
There was a problem hiding this comment.
Good point, thanks! Done.
| ├── controllers/ # Kubernetes controller implementations | ||
| │ ├── istio/ # Top-level Istio CR controller | ||
| │ ├── istiorevision/ # Revision lifecycle via Helm charts | ||
| │ ├── istiorevisiontag/ # Revision traffic routing |
There was a problem hiding this comment.
Revision traffic routing might be a bit confusing?
There was a problem hiding this comment.
I totally agree. I changed the description to "Revision tag management".
| ├── enhancements/ # Sail Enhancement Proposals (SEPs) | ||
| ├── docs/ # Documentation | ||
| ├── common/ # Shared Makefile infrastructure (from istio/common-files) | ||
| ├── ossm/ # OpenShift Service Mesh downstream code |
There was a problem hiding this comment.
Do we want to include this to upstream doc when the dir is not there?
There was a problem hiding this comment.
Sorry... I started in midstream, and this is leftover from the previous source code. I removed it.
| ## Project Structure | ||
|
|
||
| ``` | ||
| sail-operator/ |
There was a problem hiding this comment.
nit: maybe sort it alphabetically so it matches order visible in GH UI
| Profile overlay (default, openshift, demo, ambient, etc.) | ||
| │ | ||
| ▼ | ||
| Image digest injection (from config.properties) |
There was a problem hiding this comment.
Do we want to include this? I don't think an user can touch config.properties right?
There was a problem hiding this comment.
Yes, users do not touch it, but the diagram is for an agent, so I think it's valuable information. Could we leave it as is?
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jacek Ewertowski <jacek.ewertowski1@gmail.com>
| ``` | ||
| ┌─────────────────────────────────────────────┐ | ||
| │ Kubernetes API │ | ||
| └───────────────────┬─────────────────────────┘ | ||
| │ | ||
| ┌───────────────────▼─────────────────────────┐ | ||
| │ Sail Operator (Manager) │ | ||
| │ │ | ||
| │ ┌──────────┐ ┌───────────┐ ┌─────────┐ │ | ||
| │ │ Istio │ │ IstioCNI │ │ ZTunnel │ │ | ||
| │ │Controller│ │Controller │ │Controller│ │ | ||
| │ └────┬─────┘ └─────┬─────┘ └────┬────┘ │ | ||
| │ │ │ │ │ | ||
| │ ┌────▼─────────┐ │ │ │ | ||
| │ │IstioRevision │ │ │ │ | ||
| │ │ Controller │ │ │ │ | ||
| │ └────┬─────────┘ │ │ │ | ||
| │ │ │ │ │ | ||
| │ ┌────▼──────────────▼──────────────▼────┐ │ | ||
| │ │ Helm ChartManager │ │ | ||
| │ │ (install/upgrade embedded charts) │ │ | ||
| │ └────┬─────────────────────────────────┘ │ | ||
| │ │ │ | ||
| └───────┼─────────────────────────────────────┘ | ||
| │ | ||
| ┌───────▼─────────────────────────────────────┐ | ||
| │ Deployed Istio Components │ | ||
| │ (istiod, CNI DaemonSet, ZTunnel, gateways) │ | ||
| └─────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
This is misaligned..
Also you can use something like mermaid to generate an svg diagram (supported by GH) - not just here
|
LGTM, not approving so others can review too. |
| Chart defaults (values.yaml from embedded chart) | ||
| │ | ||
| ▼ | ||
| Profile overlay (default, openshift, demo, ambient, etc.) |
There was a problem hiding this comment.
The "Image digest injection" comes before "ApplyProfilesAndPlatform"
See
sail-operator/pkg/revision/values.go
Line 41 in f53307f
Also, there is an additional step where we apply the vendor specific defaults.
sail-operator/pkg/revision/values.go
Line 45 in f53307f
fjglira
left a comment
There was a problem hiding this comment.
it looks good to me, adding a hold label until comments from others reviewers are addressed
|
Hey @jewertow can we check the latest comment to see if we can merge the PR |
No description provided.