-
Notifications
You must be signed in to change notification settings - Fork 607
conformance: add Agentgateway and Istio report for v1.4.0 #4209
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @howardjohn!
| organization: istio | ||
| project: istio | ||
| url: https://istio.io/ | ||
| version: "1.28" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like v1.28 isn't out quite yet?
| organization: agentgateway | ||
| project: agentgateway | ||
| url: github.com/agentgateway/agentgateway | ||
| version: v2.2.0-main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does -main mean in the version here? Is this a fixed target or something that will change over time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will change over time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So checking over https://github.com/agentgateway/agentgateway, I see a recent v0.10.5, this versioning seems related to kgateway, not to agentgateway, and even then it's not released yet, so is this a draft PR, there will be some release we're waiting on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I'm not aware of a requirement on conformance reports around how the project handles versioning or naming. This aligns with how we want the project represented to avoid confusion with the existing Kgateway-with-Envoy implementation, and how the existing 1.3 conformance reports were published. The implementation can be run today and the conformance test is reproducible.
If there are some guidelines we are required to meet can you point me to them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I read over the reproduction steps and I think I get it now. So there is https://github.com/agentgateway/agentgateway which stands alone and that's what's in the report, but that's not exactly what this report is for, this is for kgateway which now uses agentgateway as a dependency, correct?
We have a concept of modes in the reporting structure, it sounds like this should be reported as a mode for kgateway. Then we can add the report for Agent Gateway itself as a new separate report.
So this helps me understand better where the v2.2.0-main version is coming from, because that's referring to the next anticipated release of kgateway. However, I couldn't find a tag for this on the repository, though I did find it's the moniker for some builds here. In our reporting structure we specify that the version can't be a branch name. This is to reinforce the meaning of version here to be a snapshot, point-in-time (in git, usually). So the tag v2.2.0-main should be pushed up for whichever commit this was, or we can just put the commit SHA here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way the project would prefer to position it as is that it is Agentgateway with mode Kgateway rather than the inverse. This is also how it's described on the implementation list (Agentgateway with Kgateway). However, since there is currently only kgateway we don't need split modes so just simplify it to Kgateway.
On the versioning, thanks for the reference as I hadn't seen that. I definitely agree this isn't great. I'm actually planning to clean up our versioning and expect it to be done early next week and can fix up the report. Given I'm OOO today and the reports are due Monday it would be ideal if we could merge and do a fast followup. We do have a published release at https://github.com/kgateway-dev/kgateway/pkgs/container/kgateway/561644316?tag=v2.2.0-main, and our documentation tells users to use this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok actually got this resolved pretty quickly. Pushed up a new commit. Thanks @shaneutt
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: howardjohn, robscott 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:
Approvers can indicate their approval by writing |
cf2fe03 to
ec7538d
Compare
| organization: agentgateway | ||
| project: agentgateway | ||
| url: github.com/agentgateway/agentgateway | ||
| version: v2.2.0-main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So checking over https://github.com/agentgateway/agentgateway, I see a recent v0.10.5, this versioning seems related to kgateway, not to agentgateway, and even then it's not released yet, so is this a draft PR, there will be some release we're waiting on?
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: