feat(inspect): api for retrieving service hostnames #11865
Conversation
jakubdyszkiewicz
added
the
ci/skip-test
jakubdyszkiewicz
force-pushed
the
inspect-hostnames
branch
2 times, most recently
from
fc4908d
to
b1294d2
Compare
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
jakubdyszkiewicz
force-pushed
the
inspect-hostnames
branch
from
b1294d2
to
3536693
Compare
jakubdyszkiewicz
requested review from
michaelbeaumont and
slonka
and removed request for
a team
| @@ -172,6 +172,41 @@ paths: | |||
| $ref: '#/components/responses/BadRequest' | |||
| '500': | |||
| $ref: '#/components/responses/InternalServerError' | |||
| /meshes/{mesh}/{serviceType}/{serviceName}/_resources/hostnames: | |||
There was a problem hiding this comment.
I'm not sure about this path. What _resources stand for?
@lahabana any input here?
There was a problem hiding this comment.
_resources are meant to map to other resources. For example you get /_resources/dataplanes or /_resources/policies here this doesn't look right.
IMO what you want here is: /meshes/{mesh}/{serviceType}/{serviceName}/_hostnames
jakubdyszkiewicz
removed
the
ci/skip-test
| @@ -172,6 +172,41 @@ paths: | |||
| $ref: '#/components/responses/BadRequest' | |||
| '500': | |||
| $ref: '#/components/responses/InternalServerError' | |||
| /meshes/{mesh}/{serviceType}/{serviceName}/_resources/hostnames: | |||
There was a problem hiding this comment.
_resources are meant to map to other resources. For example you get /_resources/dataplanes or /_resources/policies here this doesn't look right.
IMO what you want here is: /meshes/{mesh}/{serviceType}/{serviceName}/_hostnames
| description: The mesh the service is part of | ||
| - in: path | ||
| name: serviceType | ||
| example: meshservices |
There was a problem hiding this comment.
You should be able to add validation with just the list of supported values.
| InspectHostname: | ||
| type: object | ||
| title: InspectHostname | ||
| description: A list of zones in which the hostname is available in |
There was a problem hiding this comment.
This description seems to be the one of the zones field not of this entire object.
I think this is more
| description: A list of zones in which the hostname is available in | |
| description: An supported hostname along with the zones it exists in |
| InspectHostnameZone: | ||
| type: object | ||
| title: InspectHostnameZone | ||
| description: A name of the zone with hostname generator that was used to generate the hostname |
There was a problem hiding this comment.
I don't understand this sentence.
| request.Request.Context(), | ||
| response, | ||
| &validators.ValidationError{}, | ||
| fmt.Sprintf("only %s are available for inspection", strings.Join(util_maps.SortedKeys(typeDescForType), ",")), |
There was a problem hiding this comment.
| fmt.Sprintf("only %s are available for inspection", strings.Join(util_maps.SortedKeys(typeDescForType), ",")), | |
| fmt.Sprintf("only %q are available for inspection", strings.Join(util_maps.SortedKeys(typeDescForType), ",")), |
Motivation
We want to see hostnames of Mesh*Service in the Global CP.
Implementation information
While we could add new KDSService to retrieve stats of a resource in specific zones and then aggregate this an easier (less expensive to execute, faster, don't depend on the zone) option is to calculate the hostname. We have all the resources in global CP already (because HostnameGenerator from zone is synced to global for visibility). It's just a matter of relabeling properly.
Supporting documentation
Fix #10618