Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List revisions for a given service #194

Merged
merged 5 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion docs/cmd/kn_revision_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,23 @@ List available revisions.

### Synopsis

List available revisions.
List revisions for a given service.

```
kn revision list [flags]
```

### Examples

```

# List all revisions
kn revision list

# List revisions for a service 'svc1' in namespace 'myapp'
kn revision list -s svc1 -n myapp
```

### Options

```
Expand All @@ -18,6 +29,7 @@ kn revision list [flags]
-h, --help help for list
-n, --namespace string List the requested object(s) in given namespace.
-o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file.
-s, --service string Service name
--template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
```

Expand Down
22 changes: 21 additions & 1 deletion pkg/kn/commands/revision/revision_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import (
"fmt"

"github.com/knative/client/pkg/kn/commands"
"github.com/knative/serving/pkg/apis/serving"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
)

Expand All @@ -30,6 +32,13 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command {
revisionListCommand := &cobra.Command{
Use: "list",
Short: "List available revisions.",
Long: "List revisions for a given service.",
Example: `
# List all revisions
kn revision list

# List revisions for a service 'svc1' in namespace 'myapp'
kn revision list -s svc1 -n myapp`,
RunE: func(cmd *cobra.Command, args []string) error {
client, err := p.ServingFactory()
if err != nil {
Expand All @@ -39,7 +48,18 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command {
if err != nil {
return err
}
revision, err := client.Revisions(namespace).List(v1.ListOptions{})
listOptions := v1.ListOptions{}
if cmd.Flags().Changed("service") {
service := cmd.Flag("service").Value.String()
// Ensure requested service exist
_, err := client.Services(namespace).Get(service, v1.GetOptions{})
if err != nil {
return err
}
listOptions.LabelSelector = labels.Set(
map[string]string{serving.ConfigurationLabelKey: service}).String()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be serving.ServiceLabelKey ? Don't really see how this is tested in the tests, too ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I checked an actual revision and both the keys in labels are holding service name

                "labels": {
                    "serving.knative.dev/configuration": "svc1",
                    "serving.knative.dev/configurationGeneration": "1",
                    "serving.knative.dev/service": "svc1"
                },

@rhuss : I will need to check among 2 labels, which one we should filter against.

in tests here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't the service label the authorative answer for which revision belongs to which service and the configuration is just (by accident) named like the service because it has been created implicitely ? Or does the configuration alway be named like the service ? (I don't think so)

}
revision, err := client.Revisions(namespace).List(listOptions)
if err != nil {
return err
}
Expand Down
16 changes: 15 additions & 1 deletion pkg/kn/commands/revision/revision_list_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
type RevisionListFlags struct {
GenericPrintFlags *genericclioptions.PrintFlags
HumanReadableFlags *commands.HumanPrintFlags
ServiceRefFlags ServiceReferenceFlags
}

// AllowedFormats is the list of formats in which data can be displayed
Expand Down Expand Up @@ -55,10 +56,12 @@ func (f *RevisionListFlags) ToPrinter() (hprinters.ResourcePrinter, error) {
}

// AddFlags receives a *cobra.Command reference and binds
// flags related to humanreadable and template printing.
// flags related to humanreadable and template printing
// as well as to reference a service
func (f *RevisionListFlags) AddFlags(cmd *cobra.Command) {
f.GenericPrintFlags.AddFlags(cmd)
f.HumanReadableFlags.AddFlags(cmd)
f.ServiceRefFlags.SetOptional(cmd)
}

// NewRevisionListFlags returns flags associated with humanreadable,
Expand All @@ -69,3 +72,14 @@ func NewRevisionListFlags() *RevisionListFlags {
HumanReadableFlags: commands.NewHumanPrintFlags(),
}
}

// ServiceReferenceFlags compose a set of flag(s) to reference a service
type ServiceReferenceFlags struct {
Name string
}

// SetOptional receives a *cobra.Command reference and
// adds the ServiceReferenceFlags flags as optional flags
func (s *ServiceReferenceFlags) SetOptional(cmd *cobra.Command) {
cmd.Flags().StringVarP(&s.Name, "service", "s", "", "Service name")
}
46 changes: 45 additions & 1 deletion pkg/kn/commands/revision/revision_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
func fakeRevisionList(args []string, response *v1alpha1.RevisionList) (action client_testing.Action, output []string, err error) {
knParams := &commands.KnParams{}
cmd, fakeServing, buf := commands.CreateTestKnCommand(NewRevisionCommand(knParams), knParams)
fakeServing.AddReactor("*", "*",
fakeServing.AddReactor("list", "*",
func(a client_testing.Action) (bool, runtime.Object, error) {
action = a
return true, response, nil
Expand Down Expand Up @@ -76,6 +76,50 @@ func TestRevisionListDefaultOutput(t *testing.T) {
testContains(t, output[2], []string{"bar", "bar-wxyz"}, "value")
}

func TestRevisionListForService(t *testing.T) {
revision1 := createMockRevisionWithParams("foo-abcd", "svc1")
revision2 := createMockRevisionWithParams("bar-wxyz", "svc1")
revision3 := createMockRevisionWithParams("foo-abcd", "svc2")
revision4 := createMockRevisionWithParams("bar-wxyz", "svc2")
RevisionList := &v1alpha1.RevisionList{Items: []v1alpha1.Revision{*revision1, *revision2, *revision3, *revision4}}
action, output, err := fakeRevisionList([]string{"revision", "list", "-s", "svc1"}, RevisionList)
if err != nil {
t.Fatal(err)
}
if action == nil {
t.Errorf("No action")
} else if !action.Matches("list", "revisions") {
t.Errorf("Bad action %v", action)
}
testContains(t, output[0], []string{"SERVICE", "NAME", "AGE", "CONDITIONS", "READY", "REASON"}, "column header")
testContains(t, output[1], []string{"svc1", "foo-abcd"}, "value")
testContains(t, output[2], []string{"svc1", "bar-wxyz"}, "value")
action, output, err = fakeRevisionList([]string{"revision", "list", "-s", "svc2"}, RevisionList)
if err != nil {
t.Fatal(err)
}
if action == nil {
t.Errorf("No action")
} else if !action.Matches("list", "revisions") {
t.Errorf("Bad action %v", action)
}
testContains(t, output[0], []string{"SERVICE", "NAME", "AGE", "CONDITIONS", "READY", "REASON"}, "column header")
testContains(t, output[1], []string{"svc2", "foo-abcd"}, "value")
testContains(t, output[2], []string{"svc2", "bar-wxyz"}, "value")
//test for non existent service
action, output, err = fakeRevisionList([]string{"revision", "list", "-s", "svc3"}, RevisionList)
if err != nil {
t.Fatal(err)
}
if action == nil {
t.Errorf("No action")
} else if !action.Matches("list", "revisions") {
t.Errorf("Bad action %v", action)
} else if !strings.Contains(output[0], "No resources found.") {
t.Errorf("Bad output %s", output[0])
}
}

func testContains(t *testing.T, output string, sub []string, element string) {
for _, each := range sub {
if !strings.Contains(output, each) {
Expand Down
29 changes: 29 additions & 0 deletions test/e2e/basic_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ func TestBasicWorkflow(t *testing.T) {
testServiceCreate(t, k, "hello")
testServiceList(t, k, "hello")
testServiceDescribe(t, k, "hello")
testServiceUpdate(t, k, "hello", []string{"--env", "TARGET=kn"})
testServiceCreate(t, k, "svc2")
testRevisionListForService(t, k, "hello")
testRevisionListForService(t, k, "svc2")
testServiceDelete(t, k, "hello")
testServiceDelete(t, k, "svc2")
testServiceListEmpty(t, k)
}

Expand Down Expand Up @@ -92,6 +97,19 @@ func testServiceList(t *testing.T, k kn, serviceName string) {
}
}

func testRevisionListForService(t *testing.T, k kn, serviceName string) {
out, err := k.RunWithOpts([]string{"revision", "list", "-s", serviceName}, runOpts{NoNamespace: false})
if err != nil {
t.Fatalf(fmt.Sprintf("Error executing 'kn revision list -s %s' command. Error: %s", serviceName, err.Error()))
}
outputLines := strings.Split(out, "\n")
for _, line := range outputLines[1:] {
if len(line) > 1 && !strings.HasPrefix(line, serviceName) {
t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting line to start with service name: %s\nFound: %s", serviceName, line))
}
}
}

func testServiceDescribe(t *testing.T, k kn, serviceName string) {
out, err := k.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false})
if err != nil {
Expand All @@ -114,6 +132,17 @@ metadata:`
}
}

func testServiceUpdate(t *testing.T, k kn, serviceName string, args []string) {
out, err := k.RunWithOpts(append([]string{"service", "update", serviceName}, args...), runOpts{NoNamespace: false})
if err != nil {
t.Fatalf(fmt.Sprintf("Error executing 'kn service update' command. Error: %s", err.Error()))
}
expectedOutput := fmt.Sprintf("Service '%s' updated", serviceName)
if !strings.Contains(out, expectedOutput) {
t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting to include:\n%s\nFound:\n%s\n", expectedOutput, out))
}
}

func testServiceDelete(t *testing.T, k kn, serviceName string) {
out, err := k.RunWithOpts([]string{"service", "delete", serviceName}, runOpts{NoNamespace: false})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ k8s.io/api/storage/v1beta1
k8s.io/apimachinery/pkg/apis/meta/v1
k8s.io/apimachinery/pkg/util/duration
k8s.io/apimachinery/pkg/apis/meta/v1beta1
k8s.io/apimachinery/pkg/labels
k8s.io/apimachinery/pkg/runtime
k8s.io/apimachinery/pkg/runtime/schema
k8s.io/apimachinery/pkg/api/resource
Expand All @@ -182,7 +183,6 @@ k8s.io/apimachinery/pkg/api/validation
k8s.io/apimachinery/pkg/runtime/serializer
k8s.io/apimachinery/pkg/types
k8s.io/apimachinery/pkg/watch
k8s.io/apimachinery/pkg/labels
k8s.io/apimachinery/pkg/conversion
k8s.io/apimachinery/pkg/fields
k8s.io/apimachinery/pkg/selection
Expand Down