initial implementation of setHeaderRoute behavior #5
Conversation
Signed-off-by: Ian Rudie <[email protected]>
Signed-off-by: Ian Rudie <[email protected]>
Signed-off-by: Ian Rudie <[email protected]>
| # managedRoutes are required for using setHeaderRoute | ||
| # routes specified here are owned by the plugin | ||
| # the plugin will create/delete these routes as needed | ||
| # do not specify routes which are already otherwise in use under this field | ||
| managedRoutes: | ||
| - name: header-canary |
There was a problem hiding this comment.
It'd be great if we could detect if a Route Table has never been affected by a Rollout so that we can error. Not quite sure something like that exists.
There was a problem hiding this comment.
Off the top of my head the best I can think of would be to warn that a managed route with the name we are meant to add was found while we're adding the route
Signed-off-by: Ian Rudie <[email protected]>
ilrudie
force-pushed
the
setHeaderRoute
branch
from
7dc70a9
to
320ced2
Compare
Signed-off-by: Ian Rudie <[email protected]>
pkg/plugin/plugin.go
Outdated
| // nothing to update, don't bother computing things | ||
| return pluginTypes.RpcError{} |
There was a problem hiding this comment.
Should we throw an error if nothing is selected? The GP API is not entirely consistent in handling this scenario today, but we've definitely had feedback that it's confusing UX for users if e.g. selectors don't match because of a missing label or typo, they're wondering why the entire resource "does nothing".
There was a problem hiding this comment.
What would this error say?
"Unable to find RouteTables which match {labelSelectors}, a matching RouteTable must already exist because the glooplatform plugin does not create new RouteTables"
There was a problem hiding this comment.
I think just the first part is probably sufficient:
Unable to find any RouteTables which match {labelSelectors}
It at least gives folks a starting point for troubleshooting. If we have enough flexibility for this to be a "warning" rather than an "error", that's even better... but IDK if we have that flexibility with this particular API
There was a problem hiding this comment.
You can select a rt by name, name/namespace or labelSelectors so for now I added a basic error with a TODO to enhance it with the search criteria used which failed to produce a satisfactory result
Signed-off-by: Ian Rudie <[email protected]>
Signed-off-by: Ian Rudie <[email protected]>
…ing a working RouteTable Signed-off-by: Ian Rudie <[email protected]>
Signed-off-by: Ian Rudie <[email protected]>
ilrudie
force-pushed
the
setHeaderRoute
branch
from
c446c4c
to
5df43ee
Compare
Signed-off-by: Ian Rudie <[email protected]>
Signed-off-by: Ian Rudie <[email protected]>
ilrudie
force-pushed
the
setHeaderRoute
branch
from
7b9367d
to
5c85bd9
Compare
adds setHeaderRoute implementation