Ensure ingress host matches route host #8151
Conversation
|
cc: @smarterclayton @openshift/networking |
|
[test] |
| p := &fakePlugin{} | ||
| c := testclient.NewSimpleFake(&routeapi.Route{}) | ||
| admitter := NewStatusAdmitter(p, c, "test") | ||
| err := admitter.HandleRoute(watch.Added, &routeapi.Route{ | ||
|
|
||
| route := routeapi.Route{ |
There was a problem hiding this comment.
This isn't really testing the same thing, please preserve the old test as well.
|
Latest patch fixes the test failures (at least locally). I didn't understand how nowFn was being used in the tests. |
| if action.GetVerb() != "update" || action.GetResource() != "routes" || action.GetSubresource() != "status" { | ||
| t.Fatalf("unexpected action: %#v", action) | ||
| } | ||
| obj1 := c.Actions()[0].(ktestclient.UpdateAction).GetObject().(*routeapi.Route) |
There was a problem hiding this comment.
s#c.Actions()[0]#action#
There was a problem hiding this comment.
Would you please explain?
There was a problem hiding this comment.
You have defined action in L450, you can use that instead of accessing via the method again.
|
I've removed the new test since the other tests are already validating the requirement for RFC3339 timestamps. |
|
cc: @ramr |
| // Return a time truncated to the second to ensure that in-memory and | ||
| // serialized timestamps can be safely compared. | ||
| func getRFC3339Timestamp() unversioned.Time { | ||
| return unversioned.Time{Time: unversioned.Now().Truncate(time.Second)} |
There was a problem hiding this comment.
There's an api call available called Rfc3339Copy which returns time at the second level precision - that should work here, no?
Basically: return unversioned.Now().Rfc3339Copy()
Ref: https://github.com/kubernetes/kubernetes/blob/master/pkg/api/unversioned/time.go#L76
The router status plugin compares ingress timestamps with cached values to detect conflict with other router instances, and was falsely detecting conflict when values with nanosecond accuracy were compared to the same values that had been serialized to and from etcd in RFC3339 format (accurate to the second). The result of a conflict was that an update to the host of a route would not be reflected in the route's ingress record. This change ensures that ingress timestamps are truncated to the second to ensure safe comparison between in-memory and serialized timestamps.
|
Evaluated for origin test up to 923a155 |
|
LGTM [merge], thanks for catching this |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2442/) (Image: devenv-rhel7_3805) |
|
Evaluated for origin merge up to 923a155 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2442/) |
The router status plugin compares ingress timestamps with cached
values to detect conflict with other router instances, and was falsely
detecting conflict when values with nanosecond accuracy were compared
to the same values that had been serialized to and from etcd in
RFC3339 format (accurate to the second). The result of a conflict was
that an update to the host of a route would not be reflected in the
route's ingress record.
This change enures that ingress timestamps are truncated to the second
to ensure safe comparison between in-memory and serialized timestamps.