Skip to content

Commit

Permalink
unique_host: HandleRoute: No error if host claimed
Browse files Browse the repository at this point in the history
Change the unique_host plugin's HandleRoute method not to return an error
because the host name is claimed.

Returning an error causes the controller to log an unhelpful error message:

    router_controller.go:244] another route has claimed this host

The plugin already logs the conflict, along with the host name and the
claimants' names and namespaces, at a higher log level and updates the
Route's status to indicate that it has been rejected because of a
preëmpting claim.  Moreover, a conflicting claim does not logically
represent an error condition for the plugin or controller, and in the
absence of such, it makes more sense return a nil error value after logging
the condition.

This commit fixes bug 1780794.

https://bugzilla.redhat.com/show_bug.cgi?id=1780794

* pkg/router/controller/unique_host.go: Do not return a gratuitous error
value for a preëmpting claim.
* pkg/router/template/plugin_test.go (TestHandleRoute): Expect a nil error
value from the unique_host plugin when handling a valid Route for which a
preëmpting claim exists.
  • Loading branch information
Miciah committed Dec 6, 2019
1 parent bc1bd94 commit 7f1c1b7
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 15 deletions.
13 changes: 0 additions & 13 deletions pkg/router/controller/unique_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routev1.Route
return p.plugin.HandleRoute(eventType, route)

case watch.Added, watch.Modified:
removed := false

var nestedErr error
changes, newRoute := p.index.Add(route)

Expand Down Expand Up @@ -179,7 +177,6 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routev1.Route
}

// we were not added because another route is covering us
removed = true
var owner *routev1.Route
if old, ok := p.index.RoutesForHost(host); ok && len(old) > 0 {
owner = old[0]
Expand All @@ -203,16 +200,6 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routev1.Route
}
}

// // ensure we pass down modifications
// if !added && !removed {
// if err := p.plugin.HandleRoute(watch.Modified, route); err != nil {
// utilruntime.HandleError(fmt.Errorf("unable to modify route %s: %v", routeName, err))
// }
// }

if removed {
return fmt.Errorf("another route has claimed this host")
}
return nestedErr

default:
Expand Down
4 changes: 2 additions & 2 deletions pkg/router/template/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,8 @@ func TestHandleRoute(t *testing.T) {
},
},
}
if err := plugin.HandleRoute(watch.Added, fooDupe2); err == nil {
t.Fatal("unexpected non-error")
if err := plugin.HandleRoute(watch.Added, fooDupe2); err != nil {
t.Fatal("unexpected error")
}

if _, ok := router.FindServiceUnit(endpointsKeyFromParts("foo", "TestService2")); ok {
Expand Down

0 comments on commit 7f1c1b7

Please sign in to comment.