Update reconciler to update observed generation when ready status is False by taragu · Pull Request #4594 · knative/serving

taragu · 2019-07-02T02:41:46Z

Proposed Changes

Update reconciler to update observed generation when ready status is False

Release Note

NONE

/cc @dgerd
/cc @mattmoor
/cc @vagababov
/cc @markusthoemmes

knative-prow-robot

@taragu: 0 warnings.

Details

In response to this:

Fixes #4561

Proposed Changes

Update reconciler to update observed generation when ready status is False

Release Note
NONE
/cc @dgerd
/cc @mattmoor
/cc @vagababov
/cc @markusthoemmes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knative-prow-robot · 2019-07-02T02:42:01Z

Hi @taragu. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

vagababov · 2019-07-02T03:25:34Z

I'd expect some unit tests to change as well...

/ok-to-test

mattmoor · 2019-07-02T13:31:50Z

 		service.Status.MarkConfigurationNotReconciled()
-	} else {
+	}
+	if config.Generation == config.Status.ObservedGeneration || (rc != nil && rc.Status == corev1.ConditionFalse) {


I suspect the bug here is that Configuration / Route aren't updating the ObservedGeneration for which they are failing.

+1

This condition will be more clear if we instead make sure that we bump "ObservedGeneration" in the Route and Configuration reconcilers when "Ready" is updated to "False".

+1
For this case, "ObservedGeneration" in the configuration reconciler is already updated.
https://github.com/knative/serving/blob/master/pkg/reconciler/configuration/configuration.go#L167
In router reconciler, "ObeservedGeneration" will not be updated until route is successfully synced.
https://github.com/knative/serving/blob/master/pkg/reconciler/route/route.go#L242
Implementation in Route and Configuration reconcilers is a little different.

mattmoor · 2019-07-05T03:53:14Z

+	rc := route.Status.GetCondition(apis.ConditionReady)
+	if rc != nil && rc.Status == corev1.ConditionFalse {
+		logger.Infof("Updating ObservedGeneration")
+		existing.Status.ObservedGeneration++


Suggested change

existing.Status.ObservedGeneration++

existing.Status.ObservedGeneration = existing.Generation

We might miss spec updates and catch up to multiple changes.

I don't think the change you have here is right. I think this should be taken care of by line 188

@mattmoor I think line 188 is updating the ObservedGeneration for service.Status, versus here we are updating the ObservedGeneration for route.Status

Oh, I misread.

Status fields for resources should only be modified by the reconciler for that resource. So this is likely the change we want, but we want it in the Route reconciler, not the Service reconciler.

mattmoor · 2019-07-05T03:53:39Z

+
+	rc := route.Status.GetCondition(apis.ConditionReady)
+	if rc != nil && rc.Status == corev1.ConditionFalse {
+		logger.Infof("Updating ObservedGeneration")


I'd remove this, it's going to be super noisy and doesn't include any state.

mattmoor · 2019-07-05T03:58:06Z

+	rc := route.Status.GetCondition(apis.ConditionReady)
+	if rc != nil && rc.Status == corev1.ConditionFalse {
+		logger.Infof("Updating ObservedGeneration")
+		existing.Status.ObservedGeneration++


I don't think the change you have here is right. I think this should be taken care of by line 188

vagababov · 2019-07-05T18:48:14Z

 }

+// MarkRouteReadyFalse marks the service `RouteReady` condition to the `False` state.
+func (ss *ServiceStatus) MarkRouteReadyFalse(reason, messageFormat string) {


I think a better name would be MarkRouteFailed.

vagababov · 2019-07-05T18:50:30Z

+	}
+}
+
+// WithRouteObservedGeneration sets the .Status.ObservedGeneration of the Route.


Suggested change

// WithRouteObservedGeneration sets the .Status.ObservedGeneration of the Route.

// WithRouteObservedGeneration propagates `.Metadata.Generation` to `.Status.ObservedGeneration`.

knative-metrics-robot · 2019-07-05T19:40:21Z

The following is the coverage report on pkg/.
Say /test pull-knative-serving-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/serving/v1alpha1/service_lifecycle.go	89.3%	86.2%	-3.1
pkg/reconciler/service/service.go	88.4%	88.6%	0.2

dgerd · 2019-07-08T19:56:20Z

+
+	rc := route.Status.GetCondition(apis.ConditionReady)
+	if rc != nil && rc.Status == corev1.ConditionFalse {
+		existing.Status.ObservedGeneration = existing.Generation


Changes to anything in Route's Status should only occur in pkg/reconciler/route/route.go

taragu · 2019-07-16T18:20:09Z

/test pull-knative-serving-build-tests

dgerd · 2019-07-16T20:03:45Z

 	// assumptions about defaulting.
 	r.SetDefaults(v1beta1.WithUpgradeViaDefaulting(ctx))
 	r.Status.InitializeConditions()
+	r.Status.ObservedGeneration = r.Generation


Okay so I spent some more time looking at this. As not all returned errors from this method result in Conditions being updated to "False" which was my original assumption. We likely have some other bugs to fix in this reconciler because of this, but to prevent scope creep we should move it back to where you originally had it.

So this should be on:

Line 229 after configureTraffic fails (where you originally had it)

Line 287 after we reconcile successfully

Sorry for the back and forth here.

dgerd · 2019-07-16T20:05:16Z

+	}
+
+	t.Logf("Creating a new Service %s.", names.Service)
+	svc, err := CreateLatestService(t, clients, *names, fopt...)


You should just use this directly in your test. I don't think the rest of this function is really necessary to accomplish what you want.

dgerd · 2019-07-16T20:08:28Z

 // track how long it took for name to get into the state checked by inState.
-func WaitForRouteState(client *test.ServingBetaClients, name string, inState func(r *v1beta1.Route) (bool, error), desc string) error {
+// ignoreNotFound indicates that we will keep polling even if the route is not found.
+func WaitForRouteState(client *test.ServingBetaClients, name string, inState func(r *v1beta1.Route) (bool, error), desc string, ignoreNotFound bool) error {


This just doesn't make sense to me. If we have already waited for the Service to be Ready=False there is no way that the Route doesn't exist yet unless something else is going wrong.

@dgerd I tried removing this change after I updated the test to use an invalid RevisionName in TrafficTarget instead of an invalid image name, and I hit the error routes.serving.knative.dev "route-not-ready-oddmxzeh" not found again https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_serving/4594/pull-knative-serving-integration-tests/1151260492208017413

taragu · 2019-07-17T14:15:29Z

Looks like the test failures aren't related to changes in this PR. @dgerd I'm not able to workaround the route not found error unless error from lastState, err = client.Routes.Get(name, metav1.GetOptions{}) is ignored.

Another test TestContainerErrorMsg has the same problem too, but with Configuration configurations.serving.knative.dev "container-error-msg-zfspfsip" not found https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_serving/4594/pull-knative-serving-integration-tests/1151485219816083456

dgerd · 2019-07-17T22:27:58Z

+	}
+
+	t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)
+	if err := v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, v1a1test.IsServiceNotReady, "ServiceIsNotReady"); err != nil {


I think this line is the problem.

v1a1test.IsServiceNotReady is not checking for Service's Ready Condition to move from Unknown -> False, but is just checking that the Service is not happy. This is going to return immediately every time and not wait.

Since this check is not waiting for the Service reconciler to run it is very likely that the Route has yet to be created which is going to cause the next wait to fail. This is why adding in the 'ignoreNotFound' is working for you.

Now this waits for [observed]generation to match and Ready to be false, which I believe is the desired end-state.

dgerd · 2019-07-17T22:30:54Z

+		},
+	})
+
+	t.Log("Creating a new Service with RouteReady == Unknown and route Ready == False")


When the Service is created all conditions are set to "Unknown" this is the default state. At no point in this test will RoutesReady be Unknown and Ready be False. The transition will go:

Service.Ready = Unknown, Service.RoutesReady = Unknown

Route gets Created

Route.Ready = False as Revision is missing

Service.RoutesReady = False as Route.Ready = False.

At the same time as 4 Service.Ready = False as one of it's sub-conditions (RoutesReady) is False

dgerd · 2019-07-17T22:31:38Z

+
+	names := test.ResourceNames{
+		Service: test.ObjectNameForTest(t),
+		Image:   "helloworld",


Use test constants. Also we are looking to remove helloworld. See #4690

Suggested change

Image: "helloworld",

Image: test.PizzaPlanet1,

dgerd · 2019-07-17T22:32:23Z

 // track how long it took for name to get into the state checked by inState.
-func WaitForRouteState(client *test.ServingAlphaClients, name string, inState func(r *v1alpha1.Route) (bool, error), desc string) error {
+// ignoreNotFound indicates that we will keep polling even if the route is not found.
+func WaitForRouteState(client *test.ServingAlphaClients, name string, inState func(r *v1alpha1.Route) (bool, error), desc string, ignoreNotFound bool) error {


Lets avoid updating this signature. See comment above for what I think is going on here.

dgerd · 2019-07-18T16:45:55Z

+		// there's no ready Revision, therefore we could receive either err message from Configuration or
+		// from Route. The error propagated from Route is not as informative to users comparing to the
+		// error propagated from Configuration, so we would like to improve the error coming from Route.
 		cond := r.Status.GetCondition(v1alpha1.ConfigurationConditionReady)


This condition is looking at "ConfigurationsReady" which should never receive Ready information from the Route. It should only contain the propagated up information from the Configuration

@dgerd Thank you so much for helping me figure this out. The test was actually check for the wrong condition. It was checking ConfigurationConditionReady (which is apis.ConditionReady) here when trying to check the Service's Configuration status. It should be ServiceConditionConfigurationsReady.

…False

dgerd

I think we are getting close. A lot of minor suggestions on the tests. Please make the changes in your own branch rather than taking the Github suggestion directly. It will make merging and fixing references easier.

dgerd · 2019-07-18T21:26:58Z

+		},
+	})
+
+	t.Log("Creating a new Service with RouteReady == Unknown and route Ready == False")


Suggested change

t.Log("Creating a new Service with RouteReady == Unknown and route Ready == False")

t.Log("Creating a new Service with an invalid traffic target.")

dgerd · 2019-07-18T21:27:39Z

+		t.Fatalf("Failed to create initial Service %q: %#v", names.Service, err)
+	}
+
+	t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)


Suggested change

t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)

t.Logf("Waiting for Service %q to transition to Ready == False.", names.Service)

dgerd · 2019-07-18T21:28:31Z

+
+	t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)
+	if err := v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, v1a1test.IsServiceNotReady, "ServiceIsNotReady"); err != nil {
+		t.Fatalf("Failed waiting for Service %q to transition to Ready false: %#v", names.Service, err)


Suggested change

t.Fatalf("Failed waiting for Service %q to transition to Ready false: %#v", names.Service, err)

t.Fatalf("Failed waiting for Service %q to transition to Ready == False: %#v", names.Service, err)

dgerd · 2019-07-18T21:31:33Z

+		t.Fatalf("Failed waiting for Service %q to transition to Ready false: %#v", names.Service, err)
+	}
+
+	t.Logf("Waiting for Route %q to transition to Ready == false.", serviceresourcenames.Route(svc))


Nit: You don't need the wait here or below as you are not waiting for this change to happen, but validating that when Service is reporting False that your sub-conditions are also false. I am fine keeping it as is because I understand the helpers are nice here, but we should update the text on the log.

Suggested change

t.Logf("Waiting for Route %q to transition to Ready == false.", serviceresourcenames.Route(svc))

t.Logf("Validating Route %q has reconciled to Ready == False.", serviceresourcenames.Route(svc))

dgerd · 2019-07-18T21:32:23Z

+	}
+
+	// Wait for RouteReady to become false
+	t.Log("Wait for the service RouteReady to be false")


Suggested change

t.Log("Wait for the service RouteReady to be false")

t.Log("Validating Service %q has reconciled to RoutesReady == False.", names.Service)

dgerd · 2019-07-18T21:33:23Z

+	t.Logf("Waiting for Route %q to transition to Ready == false.", serviceresourcenames.Route(svc))
+	// Check Route is not ready
+	if err = v1a1test.WaitForRouteState(clients.ServingAlphaClient, serviceresourcenames.Route(svc), v1a1test.IsRouteNotReady, "RouteIsNotReady"); err != nil {
+		t.Fatalf("The Route %s was marked as Ready to serve traffic but it should not be: %#v", names.Route, err)


nit: pick either %q or %s and use consistently. Also use serviceresourcenames.Route(svc) as it is what is actually being used in the Wait function.

Suggested change

t.Fatalf("The Route %s was marked as Ready to serve traffic but it should not be: %#v", names.Route, err)

t.Fatalf("The Route %q was marked as Ready to serve traffic but it should not be: %#v", serviceresourcenames.Route(svc), err)

dgerd · 2019-07-18T22:09:02Z

+	// Wait for RouteReady to become false
+	t.Log("Wait for the service RouteReady to be false")
+	if err = v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, v1a1test.IsServiceNotRouteReady, "ServiceRouteReadyFalse"); err != nil {
+		t.Fatalf("Service RouteReady is not set to false")


Add service name and error. Use consistent styling.

Suggested change

t.Fatalf("Service RouteReady is not set to false")

t.Fatalf("Service %q was not marked RouteReady == False: %#v, names.Service, err)

dgerd · 2019-07-18T22:10:39Z


-// IsServiceNotReady will check the status conditions of the service and return true if the service is
-// not ready.
+// IsServiceNotReady will check the status conditions of the service and return false if the service is


Don't take take the suggestion straight from github as the line bleeds over, but use consistent style with below.

Suggested change

// IsServiceNotReady will check the status conditions of the service and return false if the service is

// IsServiceNotReady checks the Ready status condition of the service and returns true only if Ready is set to False.

dgerd · 2019-07-18T22:12:37Z

+	return result != nil && result.Status == corev1.ConditionFalse, nil
+}
+
+// IsServiceNotRouteReady checks the RouteReady status of the service and return false if the service's


Suggested change

// IsServiceNotRouteReady checks the RouteReady status of the service and return false if the service's

// IsServiceNotRouteReady checks the RouteReady status of the service and returns true only if RoutesReady is set to False.

dgerd · 2019-07-18T22:13:40Z

+
+// IsServiceNotRouteReady checks the RouteReady status of the service and return false if the service's
+// RouteReady is true.
+func IsServiceNotRouteReady(s *v1alpha1.Service) (bool, error) {


nit: The condition is actually "RoutesReady" it makes more sense grammatically if the "not' is moved.

Suggested change

func IsServiceNotRouteReady(s *v1alpha1.Service) (bool, error) {

func IsServiceRoutesNotReady(s *v1alpha1.Service) (bool, error) {

mattmoor · 2019-07-19T00:37:56Z

 func IsServiceNotReady(s *v1alpha1.Service) (bool, error) {
-	return s.Generation == s.Status.ObservedGeneration && !s.Status.IsReady(), nil
+	result := s.Status.GetCondition(v1alpha1.ServiceConditionReady)
+	return result != nil && result.Status == corev1.ConditionFalse, nil


Can we keep the observed generation check?

mattmoor · 2019-07-19T00:38:12Z

+// IsServiceRoutesNotReady checks the RoutesReady status of the service and returns true only if RoutesReady is set to False.
+func IsServiceRoutesNotReady(s *v1alpha1.Service) (bool, error) {
+	result := s.Status.GetCondition(v1alpha1.ServiceConditionRoutesReady)
+	return result != nil && result.Status == corev1.ConditionFalse, nil


Add an observed generation check?

mattmoor

thanks for your patience with this @taragu, I know it's been a bit of a minefield, but I appreciate you sticking with it.

mattmoor · 2019-07-19T01:18:45Z

+	}
+
+	t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)
+	if err := v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, v1a1test.IsServiceNotReady, "ServiceIsNotReady"); err != nil {


Now this waits for [observed]generation to match and Ready to be false, which I believe is the desired end-state.

mattmoor · 2019-07-19T01:19:25Z

+
+	t.Logf("Validating Route %q has reconciled to Ready == False.", serviceresourcenames.Route(svc))
+	// Check Route is not ready
+	if err = v1a1test.WaitForRouteState(clients.ServingAlphaClient, serviceresourcenames.Route(svc), v1a1test.IsRouteNotReady, "RouteIsNotReady"); err != nil {


At this point we should be able to replace WaitFor here with Check since the Service has already observed the bad state, so we should get that same bad state.

mattmoor · 2019-07-19T01:19:44Z

+
+	// Wait for RoutesReady to become False
+	t.Logf("Validating Service %q has reconciled to RoutesReady == False.", names.Service)
+	if err = v1a1test.WaitForServiceState(clients.ServingAlphaClient, names.Service, v1a1test.IsServiceRoutesNotReady, "ServiceRouteReadyFalse"); err != nil {


Same, use Check instead of WaitFor.

taragu · 2019-07-19T12:51:45Z

@dgerd @mattmoor thank you so much for your review comments! I've updated the PR to address them. Please take a look

dgerd · 2019-07-23T17:45:24Z

/lgtm
/approve

knative-prow-robot · 2019-07-23T17:45:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgerd, taragu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/reconciler/route/OWNERS~~ [dgerd]
~~test/OWNERS~~ [dgerd]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow-robot requested review from dgerd, markusthoemmes, mattmoor and vagababov July 2, 2019 02:41

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 2, 2019

knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 2, 2019

knative-prow-robot reviewed Jul 2, 2019

View reviewed changes

knative-prow-robot added area/API API objects and controllers needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 2, 2019

taragu mentioned this pull request Jul 2, 2019

Service's RoutesReady is 'Unknown' when Route's Ready is 'False' #4561

Closed

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 2, 2019

mattmoor reviewed Jul 2, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch from 5cf40fc to af65283 Compare July 4, 2019 16:30

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 4, 2019

mattmoor reviewed Jul 5, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch from af65283 to 23e58a8 Compare July 5, 2019 15:24

vagababov reviewed Jul 5, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch from 23e58a8 to 6c9b19e Compare July 5, 2019 19:37

dgerd reviewed Jul 8, 2019

View reviewed changes

dprotaso mentioned this pull request Jul 10, 2019

Check route is not ready when underlying revision is not healthy in TestContainerErrorMsg #4682

Merged

taragu force-pushed the fix-service-routesready-unknown branch from 6c9b19e to e465bb4 Compare July 14, 2019 01:18

knative-prow-robot added the area/networking label Jul 14, 2019

taragu force-pushed the fix-service-routesready-unknown branch from e465bb4 to e954c2d Compare July 15, 2019 13:38

taragu force-pushed the fix-service-routesready-unknown branch 2 times, most recently from d628ef6 to 3257723 Compare July 16, 2019 17:54

dgerd reviewed Jul 16, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch 3 times, most recently from 1674f30 to 8a1b852 Compare July 17, 2019 13:28

dgerd reviewed Jul 17, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch 3 times, most recently from 390dac6 to b77ad3c Compare July 18, 2019 16:14

dgerd reviewed Jul 18, 2019

View reviewed changes

Update reconciler to update observed generation when ready status is …

3ae54d9

…False

taragu force-pushed the fix-service-routesready-unknown branch from b77ad3c to 3ae54d9 Compare July 18, 2019 18:21

dgerd reviewed Jul 18, 2019

View reviewed changes

mattmoor reviewed Jul 19, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch from b0e84ec to 304a583 Compare July 19, 2019 00:48

mattmoor reviewed Jul 19, 2019

View reviewed changes

taragu force-pushed the fix-service-routesready-unknown branch from 304a583 to a22ba07 Compare July 19, 2019 02:31

Address review comments

979204d

taragu force-pushed the fix-service-routesready-unknown branch from a22ba07 to 979204d Compare July 19, 2019 13:05

knative-prow-robot assigned dgerd Jul 23, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2019

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 23, 2019

knative-prow-robot merged commit 42ea050 into knative:master Jul 23, 2019

taragu deleted the fix-service-routesready-unknown branch November 12, 2019 16:03

	existing.Status.ObservedGeneration++
	existing.Status.ObservedGeneration = existing.Generation

	// WithRouteObservedGeneration sets the .Status.ObservedGeneration of the Route.
	// WithRouteObservedGeneration propagates `.Metadata.Generation` to `.Status.ObservedGeneration`.

	t.Log("Creating a new Service with RouteReady == Unknown and route Ready == False")
	t.Log("Creating a new Service with an invalid traffic target.")

	t.Logf("Waiting for Service %q to transition to Ready == false.", names.Service)
	t.Logf("Waiting for Service %q to transition to Ready == False.", names.Service)

	t.Fatalf("Failed waiting for Service %q to transition to Ready false: %#v", names.Service, err)
	t.Fatalf("Failed waiting for Service %q to transition to Ready == False: %#v", names.Service, err)

	t.Logf("Waiting for Route %q to transition to Ready == false.", serviceresourcenames.Route(svc))
	t.Logf("Validating Route %q has reconciled to Ready == False.", serviceresourcenames.Route(svc))

	t.Log("Wait for the service RouteReady to be false")
	t.Log("Validating Service %q has reconciled to RoutesReady == False.", names.Service)

	t.Fatalf("The Route %s was marked as Ready to serve traffic but it should not be: %#v", names.Route, err)
	t.Fatalf("The Route %q was marked as Ready to serve traffic but it should not be: %#v", serviceresourcenames.Route(svc), err)

	t.Fatalf("Service RouteReady is not set to false")
	t.Fatalf("Service %q was not marked RouteReady == False: %#v, names.Service, err)

	// IsServiceNotReady will check the status conditions of the service and return false if the service is
	// IsServiceNotReady checks the Ready status condition of the service and returns true only if Ready is set to False.

	// IsServiceNotRouteReady checks the RouteReady status of the service and return false if the service's
	// IsServiceNotRouteReady checks the RouteReady status of the service and returns true only if RoutesReady is set to False.

	func IsServiceNotRouteReady(s *v1alpha1.Service) (bool, error) {
	func IsServiceRoutesNotReady(s *v1alpha1.Service) (bool, error) {

Conversation

taragu commented Jul 2, 2019

Proposed Changes

Uh oh!

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

Uh oh!

knative-prow-robot commented Jul 2, 2019

Uh oh!

vagababov commented Jul 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yu2003w Jul 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knative-metrics-robot commented Jul 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taragu commented Jul 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taragu commented Jul 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dgerd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

yu2003w Jul 3, 2019 •

edited

Loading