With sync update, get revision name with latestReadyRevisionName

[navidshaikh]

/lint

Fixes # 500 Check #506

Proposed Changes

• We got synchronous service update operation, ensuring the requested
  config change is reconciled before service update operation returns. feature(service): Wait on update for service to become ready. #271

• Traffic splitting e2e tests, do sync service update to generate the
  revisions.

• With every service update, we need to grab the revision
  name generated with this update (for subsequent operations),
  which we can grab using latestReadyRevisionName since sync service updated returned.

[knative-prow-robot]

@navidshaikh: 0 warnings.

Details

In response to this:

/lint

Fixes #500

Proposed Changes

• We got synchronous service update operation, ensuring the requested
  config change is reconciled before service update operation returns. #271

• Traffic splitting e2e tests, do sync service update to generate the
  revisions.

• With every service update, we need to grab the revision
  name generated with this update (for subsequent operations),
  which we can grab using latestReadyRevisionName since sync service updated returned.

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh

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:

• OWNERS [navidshaikh]

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

[navidshaikh]

Per #500 e2e for a test,
the operations performed are
1 - kn service create [..] <- create a revision
2 - kn service update --env [..] <- create another revision
3 - kn service update --tag <- don't create revision but only update traffic block to append a target with a tag
4 - Expected revision created in step 2 in latestRevision: true block.

actual result:

• Revision created in step 1 is set in latestRevision: true block.

The service update returned successfully, means the requested changes were reconciled, then
latestCreatedRevision should be equal to latestReadyRevision.

The failure though reports, latestCreatedRevision != latestReadyRevision

    --- FAIL: TestTrafficSplit/tag_a_revision_as_candidate,_without_otherwise_changing_any_traffic_split (7.74s)
        traffic_split_test.go:368: assertion failed: 
            --- expectedTargets
            +++ formattedActualTargets
              []e2e.TargetFields{
              	{
              		Tag:      "",
            - 		Revision: "echo2-ytdyn-2",
            + 		Revision: "echo2-qjxkx-1",
              		Percent:  100,
              		Latest:   true,
              	},
              	{Tag: "candidate", Revision: "echo2-qjxkx-1"},
              }

which means that, service didn't put correct revisionName for latestReadyRevision:true traffic block.

[navidshaikh]

We can update the traffic splitting tests to reference BYO revision names using --revision-name for create and update. This eliminates the need to grab the revision name after last create/update operation performed, this should be another test to see if the revisions names set in traffic block are as expected. Updating the PR..
/hold

[navidshaikh]

/test pull-knative-client-integration-tests-latest-release

[navidshaikh]

From the first CI run against serving nightly (pull-knative-client-integration-tests ), excerpts logs

Failed to successfully execute 
'kn service update echo1 --traffic echo1-yxfyl-1=20,echo1-yxfyl-1=80 -n kne2etests2':
Execution error: stderr: 'repetition of revision reference echo1-yxfyl-1 is not allowed, 
use only once with --traffic flag

The steps before the failed operation were:
1 - Sync kn service create

2 - Sync kn service update svcName --env [..] - reconciled

3 - rev1 := getLatestReadyRevision (using jsonpath={.status.latestReadyRevisionName})

4 - Sync kn service update svcName --env [..] - reconciled

5 - rev2 := getLatestReadyRevision (using jsonpath={.status.latestReadyRevisionName})

At this point, we expect rev1 != rev2, however per error message getLatestReadyRevision in step 2 and 4 returned same revision.

[knative-prow-robot]

@navidshaikh: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-client-integration-tests	f8da123	link	/test pull-knative-client-integration-tests

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[navidshaikh]

Beside pausing for 3 seconds to let latestReadyRevision populate the correct revision name while verifying the targets, we are also getting this issue when we extract the server side generated revision names after each update. So fix should be in #506 where we have BYO revision-names for all the operations, with wait/sleep happening only at one place.

[navidshaikh]

The needed changes are addressed in #506
/close

[knative-prow-robot]

@navidshaikh: Closed this PR.

Details

In response to this:

The needed changes are addressed in #506
/close

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.