adding SE status generation#2979
Conversation
Signed-off-by: ilrudie <ian.rudie@solo.io>
|
😊 Welcome @ilrudie! This is either your first contribution to the Istio tools repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
istio-testing
added
size/M
ilrudie
force-pushed
the
se-status-gen
branch
2 times, most recently
from
abfe363 to
fbea7d7
Compare
Signed-off-by: ilrudie <ian.rudie@solo.io>
cmd/kubetype-gen/generators/types.go
Outdated
| localM := m | ||
| // ServiceEntry has a unique status type which includes addresses for auto allocated IPs, substitute IstioServiceEntryStatus | ||
| // for IstioStatus when type is ServiceEntry | ||
| if kubeType.Type().Name.Name == "ServiceEntry" { |
There was a problem hiding this comment.
would be ideal if this was from checking some comment on the type indicating this
There was a problem hiding this comment.
potential impl in latest commit, thanks for the suggestion, I think this would allow decoupling this PR from API PR somewhat, yeah?
istio-testing
added
size/S
…nstead of matching type name Signed-off-by: ilrudie <ian.rudie@solo.io>
howardjohn
left a comment
howardjohn
left a comment
There was a problem hiding this comment.
lgtm but with a hold - either need to not hardcode here, or address my comments on the API to rename the field and align here
cmd/kubetype-gen/generators/types.go
Outdated
| // ServiceEntry has a unique status type which includes addresses for auto allocated IPs, substitute IstioServiceEntryStatus | ||
| // for IstioStatus when type is ServiceEntry | ||
| if slices.Contains(kubeType.RawType().CommentLines, useIstioServiceEntryStatus) { | ||
| localM["IstioStatus"] = c.Universe.Type(types.Name{Name: "IstioServiceEntryStatus", Package: "istio.io/api/meta/v1alpha1"}) |
There was a problem hiding this comment.
nit: extract the custom type instead of hard coding .
There was a problem hiding this comment.
added, also istio/api#3257 adds the required comment so we can remove the hardcode
Signed-off-by: ilrudie <ilrudie@gmail.com>
istio-testing
added
size/M
howardjohn
left a comment
howardjohn
left a comment
There was a problem hiding this comment.
#2982 adds another way to set the same thing, maybe we can consolidate on that. Can be a followup
We'd change to this in api and the adjust the parsing in the kubetype-gen to consume this cue-gen tag? |
|
yeah that's what I was thinking. the whole "cue-gen" prefix a bit odd, but
we don't even use "cue" anymore anyway
…On Tue, Jul 9, 2024, 10:24 AM Ian Rudie ***@***.***> wrote:
#2982 <#2982> adds another way to set
the same thing, maybe we can consolidate on that. Can be a followup
We'd change to this in api and the adjust the parsing in the kubetype-gen
to consume this cue-gen tag?
//
+cue-gen:ServiceEntry:subresource:status=istio.networking.v1alpha3.ServiceEntryStatus
—
Reply to this email directly, view it on GitHub
<#2979 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYGXOA7QGG7XEYEPBVAUTZLPXCVAVCNFSM6AAAAABKJZXELWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJXHA3TQNZYGI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
It's certainly cleaner than what I did despite the odd prefix. I considered using tags but was wary of potential knock-on effects so I went with a regular comment. |
4 participants
adds logic to consume the new ServiceEntry status from istio/api#3244 when generating our types