refactor kn service export to export revisions

[itsmurugappan]

Description

Changes made to export revisions directly with kn service export.
#728 (comment) - This was the approach followed

Changes

• --with-revisions option will export service and revision list
• --with-revisions --kubernetes-resources will export a service list (kubectl friendly)
• add annotations to export

Reference

Fixes #728

[knative-prow-robot]

@itsmurugappan: 6 warnings.

In response to this:

Description

Changes made to export revisions directly with kn service export.
#728 (comment) - This was the approach followed

Changes

• --with-revisions option will export service and revision list
• --with-revisions --kubernetes-resources will export a service list (kubectl friendly)
• add annotations to export

Reference

Fixes #728

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.

[itsmurugappan]

/assign @rhuss

[maximilien]

Any way to have the expected YAML in the tests? That way it’s clear and just compares output from test to YAML itself?

[itsmurugappan]

Any way to have the expected YAML in the tests? That way it’s clear and just compares output from test to YAML itself?

like a set of expected YAML's in a folder and compare against them ? I think for e2e we can do them. @maximilien

[rhuss]

Thanks a lot ! I was a bit busy the last week, but will work on the review soon

[rhuss]

Thanks a lot for your PR, it looks good to me with some minor change requests (see below).

Besides this, I wonder whether instead of exporting a service and a revision list we might should introduce a new type KnExport with fields service: and revisions: to embed the service and the revisions. I'm not sure how easy this is (i.e. how to add this as schema so that cli-runtime/pkg/printers can use this too.), but the big advantage of an own type is that it's then crystal clear that the export can not be applied with kubectl but with kn import only (so that the owneReferences can be fixed).

wdyt ? We can also do it in two steps: First merge this PR and the think about that KnExport (or KnativeExport type).

[itsmurugappan]

wdyt ? We can also do it in two steps: First merge this PR and the think about that KnExport (or KnativeExport type).

@rhuss I also think its an excellent idea to introduce a new type. It will be a much cleaner approach. Also made the changes requested by you.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/kn/commands/service/export.go	82.3%	85.2%	2.9

[rhuss]

Thanks ! Lookas good to me. Let's merge it now and then think about an own export format. I opened #824 to track this.

/lgtm

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: itsmurugappan, rhuss

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhuss]

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