From 65558aca78c48187fc288d6b14610359197f5687 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Thu, 22 Oct 2015 11:20:37 -0400 Subject: [PATCH] unified printing strategy --- contrib/completions/bash/oadm | 12 ++++ contrib/completions/bash/openshift | 12 ++++ pkg/cmd/admin/groups/new.go | 20 +++++- pkg/cmd/admin/groups/sync/cli/prune.go | 7 ++ pkg/cmd/admin/groups/sync/cli/sync.go | 30 +++----- pkg/cmd/admin/groups/sync/grouppruner.go | 12 +++- pkg/cmd/admin/groups/sync/grouppruner_test.go | 2 + pkg/cmd/admin/groups/sync/groupsyncer.go | 27 +++++-- pkg/cmd/admin/groups/sync/groupsyncer_test.go | 12 ++-- pkg/cmd/admin/project/new_project.go | 11 ++- pkg/cmd/cli/secrets/basicauth.go | 10 ++- pkg/cmd/cli/secrets/dockercfg.go | 11 ++- pkg/cmd/cli/secrets/new.go | 11 ++- pkg/cmd/cli/secrets/sshauth.go | 10 ++- pkg/cmd/util/mutation/mutate.go | 70 +++++++++++++++++++ test/integration/groups_test.go | 8 ++- test/integration/login_test.go | 10 +-- test/util/server/server.go | 3 + 18 files changed, 227 insertions(+), 51 deletions(-) create mode 100644 pkg/cmd/util/mutation/mutate.go diff --git a/contrib/completions/bash/oadm b/contrib/completions/bash/oadm index a839476a0578..9876170015ce 100644 --- a/contrib/completions/bash/oadm +++ b/contrib/completions/bash/oadm @@ -188,6 +188,12 @@ _oadm_new-project() flags+=("--description=") flags+=("--display-name=") flags+=("--node-selector=") +<<<<<<< HEAD +======= + flags+=("--output=") + two_word_flags+=("-o") + flags+=("--alsologtostderr") +>>>>>>> unified printing strategy flags+=("--api-version=") flags+=("--certificate-authority=") flags_with_completion+=("--certificate-authority") @@ -1038,6 +1044,12 @@ _oadm_groups_new() flags_with_completion=() flags_completion=() +<<<<<<< HEAD +======= + flags+=("--output=") + two_word_flags+=("-o") + flags+=("--alsologtostderr") +>>>>>>> unified printing strategy flags+=("--api-version=") flags+=("--certificate-authority=") flags_with_completion+=("--certificate-authority") diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index 1fc06a6a6c98..5d781c8f1ccc 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -743,6 +743,12 @@ _openshift_admin_new-project() flags+=("--description=") flags+=("--display-name=") flags+=("--node-selector=") +<<<<<<< HEAD +======= + flags+=("--output=") + two_word_flags+=("-o") + flags+=("--alsologtostderr") +>>>>>>> unified printing strategy flags+=("--api-version=") flags+=("--certificate-authority=") flags_with_completion+=("--certificate-authority") @@ -1593,6 +1599,12 @@ _openshift_admin_groups_new() flags_with_completion=() flags_completion=() +<<<<<<< HEAD +======= + flags+=("--output=") + two_word_flags+=("-o") + flags+=("--alsologtostderr") +>>>>>>> unified printing strategy flags+=("--api-version=") flags+=("--certificate-authority=") flags_with_completion+=("--certificate-authority") diff --git a/pkg/cmd/admin/groups/new.go b/pkg/cmd/admin/groups/new.go index e923f6c85af2..8a73eb829828 100644 --- a/pkg/cmd/admin/groups/new.go +++ b/pkg/cmd/admin/groups/new.go @@ -5,13 +5,14 @@ import ( "fmt" "io" + "github.com/spf13/cobra" + kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/util/sets" - "github.com/spf13/cobra" - "github.com/openshift/origin/pkg/client" "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/mutation" userapi "github.com/openshift/origin/pkg/user/api" ) @@ -34,6 +35,9 @@ type NewGroupOptions struct { Group string Users []string + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } func NewCmdNewGroup(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command { @@ -49,10 +53,14 @@ func NewCmdNewGroup(name, fullName string, f *clientcmd.Factory, out io.Writer) kcmdutil.CheckErr(kcmdutil.UsageError(cmd, err.Error())) } + options.MutationOutputOptions = mutation.NewMutationOutputOptions(f.Factory, cmd, out) + kcmdutil.CheckErr(options.AddGroup()) }, } + kcmdutil.AddOutputFlagsForMutation(cmd) + return cmd } @@ -90,5 +98,11 @@ func (o *NewGroupOptions) AddGroup() error { } _, err := o.GroupClient.Create(group) - return err + if err != nil { + return err + } + if err := o.MutationOutputOptions.PrintSuccess(group, "created"); err != nil { + return err + } + return nil } diff --git a/pkg/cmd/admin/groups/sync/cli/prune.go b/pkg/cmd/admin/groups/sync/cli/prune.go index d692fd4e3754..873a02b45f99 100644 --- a/pkg/cmd/admin/groups/sync/cli/prune.go +++ b/pkg/cmd/admin/groups/sync/cli/prune.go @@ -19,6 +19,7 @@ import ( "github.com/openshift/origin/pkg/cmd/server/api" "github.com/openshift/origin/pkg/cmd/server/api/validation" "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/mutation" ) const ( @@ -70,6 +71,9 @@ type PruneOptions struct { // Out is the writer to write output to Out io.Writer + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } func NewPruneOptions() *PruneOptions { @@ -97,6 +101,8 @@ func NewCmdPrune(name, fullName string, f *clientcmd.Factory, out io.Writer) *co cmdutil.CheckErr(cmdutil.UsageError(c, err.Error())) } + options.MutationOutputOptions = mutation.NewMutationOutputOptions(f.Factory, c, options.Out) + if err := options.Validate(); err != nil { cmdutil.CheckErr(cmdutil.UsageError(c, err.Error())) } @@ -188,6 +194,7 @@ func (o *PruneOptions) Run(cmd *cobra.Command, f *clientcmd.Factory) error { Out: o.Out, Err: os.Stderr, + MutationOutputOptions: o.MutationOutputOptions, } listerMapper, err := getOpenShiftGroupListerMapper(clientConfig.Host(), o) diff --git a/pkg/cmd/admin/groups/sync/cli/sync.go b/pkg/cmd/admin/groups/sync/cli/sync.go index 7b3cb951253c..69421c9f54ea 100644 --- a/pkg/cmd/admin/groups/sync/cli/sync.go +++ b/pkg/cmd/admin/groups/sync/cli/sync.go @@ -10,7 +10,6 @@ import ( "github.com/spf13/cobra" - kapi "k8s.io/kubernetes/pkg/api" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/runtime" kerrs "k8s.io/kubernetes/pkg/util/errors" @@ -27,8 +26,8 @@ import ( "github.com/openshift/origin/pkg/cmd/server/api" configapilatest "github.com/openshift/origin/pkg/cmd/server/api/latest" "github.com/openshift/origin/pkg/cmd/server/api/validation" - ocmdutil "github.com/openshift/origin/pkg/cmd/util" "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/mutation" ) const ( @@ -102,6 +101,9 @@ type SyncOptions struct { // Out is the writer to write output to Out io.Writer + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } // CreateErrorHandler creates an error handler for the LDAP sync job @@ -145,6 +147,8 @@ func NewCmdSync(name, fullName string, f *clientcmd.Factory, out io.Writer) *cob cmdutil.CheckErr(cmdutil.UsageError(c, err.Error())) } + options.MutationOutputOptions = mutation.NewMutationOutputOptions(f.Factory, c, options.Out) + if err := options.Validate(); err != nil { cmdutil.CheckErr(cmdutil.UsageError(c, err.Error())) } @@ -357,6 +361,7 @@ func (o *SyncOptions) Run(cmd *cobra.Command, f *clientcmd.Factory) error { Out: o.Out, Err: os.Stderr, + MutationOutputOptions: o.MutationOutputOptions, } switch o.Source { @@ -394,26 +399,7 @@ func (o *SyncOptions) Run(cmd *cobra.Command, f *clientcmd.Factory) error { return err } - // Now we run the Syncer and report any errors - openshiftGroups, syncErrors := syncer.Sync() - if o.Confirm { - return kerrs.NewAggregate(syncErrors) - } - - list := &kapi.List{} - for _, item := range openshiftGroups { - list.Items = append(list.Items, item) - } - list.Items, err = ocmdutil.ConvertItemsForDisplayFromDefaultCommand(cmd, list.Items) - if err != nil { - return err - } - - if err := f.Factory.PrintObject(cmd, list, o.Out); err != nil { - return err - } - - return kerrs.NewAggregate(syncErrors) + return kerrs.NewAggregate(syncer.Sync()) } func buildSyncBuilder(clientConfig ldapclient.Config, syncConfig *api.LDAPSyncConfig, errorHandler syncerror.Handler) (SyncBuilder, error) { diff --git a/pkg/cmd/admin/groups/sync/grouppruner.go b/pkg/cmd/admin/groups/sync/grouppruner.go index cf295638d266..9f2e86948845 100644 --- a/pkg/cmd/admin/groups/sync/grouppruner.go +++ b/pkg/cmd/admin/groups/sync/grouppruner.go @@ -8,6 +8,7 @@ import ( "github.com/openshift/origin/pkg/client" "github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" + "github.com/openshift/origin/pkg/cmd/util/mutation" ) // GroupPruner runs a prune job on Groups @@ -33,6 +34,9 @@ type LDAPGroupPruner struct { // Out is used to provide output while the sync job is happening Out io.Writer Err io.Writer + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } var _ GroupPruner = &LDAPGroupPruner{} @@ -79,7 +83,13 @@ func (s *LDAPGroupPruner) Prune() []error { } } - fmt.Fprintf(s.Out, "group/%s\n", groupName) + // The entire prune operation is not atomic, so we need to output what we do when we do it, so if we encounter + // a failure on another group the work we have already done is accounted for. + if s.DryRun { + s.MutationOutputOptions.PrintSuccessForDescription("Group", groupName, "would be deleted") + } else { + s.MutationOutputOptions.PrintSuccessForDescription("Group", groupName, "deleted") + } } return errors diff --git a/pkg/cmd/admin/groups/sync/grouppruner_test.go b/pkg/cmd/admin/groups/sync/grouppruner_test.go index 77fb06f89650..271c8c384d61 100644 --- a/pkg/cmd/admin/groups/sync/grouppruner_test.go +++ b/pkg/cmd/admin/groups/sync/grouppruner_test.go @@ -12,6 +12,7 @@ import ( "github.com/openshift/origin/pkg/client/testclient" "github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" + "github.com/openshift/origin/pkg/cmd/util/mutation" ) func TestGoodPrune(t *testing.T) { @@ -121,6 +122,7 @@ func newTestPruner() (*LDAPGroupPruner, *testclient.Fake) { Host: newTestHost(), Out: ioutil.Discard, Err: ioutil.Discard, + MutationOutputOptions: mutation.NewFakeOptions(), }, tc } diff --git a/pkg/cmd/admin/groups/sync/groupsyncer.go b/pkg/cmd/admin/groups/sync/groupsyncer.go index 3a6a03d837e4..5e24258f211b 100644 --- a/pkg/cmd/admin/groups/sync/groupsyncer.go +++ b/pkg/cmd/admin/groups/sync/groupsyncer.go @@ -14,13 +14,14 @@ import ( "github.com/openshift/origin/pkg/auth/ldaputil" "github.com/openshift/origin/pkg/client" "github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" + "github.com/openshift/origin/pkg/cmd/util/mutation" userapi "github.com/openshift/origin/pkg/user/api" ) // GroupSyncer runs a Sync job on Groups type GroupSyncer interface { // Sync syncs groups in OpenShift with records from an external source - Sync() (groupsAffected []*userapi.Group, errors []error) + Sync() (errors []error) } // LDAPGroupSyncer sync Groups with records on an external LDAP server @@ -43,13 +44,15 @@ type LDAPGroupSyncer struct { // Out is used to provide output while the sync job is happening Out io.Writer Err io.Writer + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } var _ GroupSyncer = &LDAPGroupSyncer{} // Sync allows the LDAPGroupSyncer to be a GroupSyncer -func (s *LDAPGroupSyncer) Sync() ([]*userapi.Group, []error) { - openshiftGroups := []*userapi.Group{} +func (s *LDAPGroupSyncer) Sync() []error { var errors []error // determine what to sync @@ -57,7 +60,7 @@ func (s *LDAPGroupSyncer) Sync() ([]*userapi.Group, []error) { ldapGroupUIDs, err := s.GroupLister.ListGroups() if err != nil { errors = append(errors, err) - return nil, errors + return errors } glog.V(1).Infof("Sync ldapGroupUIDs %v", ldapGroupUIDs) @@ -88,19 +91,29 @@ func (s *LDAPGroupSyncer) Sync() ([]*userapi.Group, []error) { errors = append(errors, err) continue } - openshiftGroups = append(openshiftGroups, openshiftGroup) if !s.DryRun { - fmt.Fprintf(s.Out, "group/%s\n", openshiftGroup.Name) if err := s.updateOpenShiftGroup(openshiftGroup); err != nil { fmt.Fprintf(s.Err, "Error updating OpenShift group %q for LDAP group %q: %v.\n", openshiftGroup.Name, ldapGroupUID, err) errors = append(errors, err) continue } } + + // The entire sync operation is not atomic, so we need to output what we do when we do it, so if we encounter + // a failure on another group the work we have already done is accounted for. + if s.DryRun { + if err := s.MutationOutputOptions.PrintSuccess(openshiftGroup, "would be updated"); err != nil { + errors = append(errors, err) + } + } else { + if err := s.MutationOutputOptions.PrintSuccess(openshiftGroup, "updated"); err != nil { + errors = append(errors, err) + } + } } - return openshiftGroups, errors + return errors } // determineUsers determines the OpenShift Users that correspond to a list of LDAP member entries diff --git a/pkg/cmd/admin/groups/sync/groupsyncer_test.go b/pkg/cmd/admin/groups/sync/groupsyncer_test.go index 0468e6e057a7..dae0583613c3 100644 --- a/pkg/cmd/admin/groups/sync/groupsyncer_test.go +++ b/pkg/cmd/admin/groups/sync/groupsyncer_test.go @@ -16,6 +16,7 @@ import ( "github.com/openshift/origin/pkg/auth/ldaputil" "github.com/openshift/origin/pkg/client/testclient" "github.com/openshift/origin/pkg/cmd/admin/groups/sync/interfaces" + "github.com/openshift/origin/pkg/cmd/util/mutation" userapi "github.com/openshift/origin/pkg/user/api" ) @@ -190,7 +191,7 @@ var Group3Members []*ldap.Entry = []*ldap.Entry{Member3, Member4} // TestGoodSync ensures that data is exchanged and rearranged correctly during the sync process. func TestGoodSync(t *testing.T) { testGroupSyncer, tc := newTestSyncer() - _, errs := testGroupSyncer.Sync() + errs := testGroupSyncer.Sync() for _, err := range errs { t.Errorf("unexpected sync error: %v", err) } @@ -202,24 +203,20 @@ func TestListFails(t *testing.T) { testGroupSyncer, _ := newTestSyncer() testGroupSyncer.GroupLister.(*TestGroupLister).err = errors.New("error during listing") - groups, errs := testGroupSyncer.Sync() + errs := testGroupSyncer.Sync() if len(errs) != 1 { t.Errorf("unexpected sync error: %v", errs) } else if errs[0] != testGroupSyncer.GroupLister.(*TestGroupLister).err { t.Errorf("unexpected sync error: %v", errs) } - - if groups != nil { - t.Errorf("unexpected groups %v", groups) - } } func TestMissingLDAPGroupUIDMapping(t *testing.T) { testGroupSyncer, tc := newTestSyncer() testGroupSyncer.GroupLister.(*TestGroupLister).GroupUIDs = append(testGroupSyncer.GroupLister.(*TestGroupLister).GroupUIDs, "ldapgroupwithnouid") - _, errs := testGroupSyncer.Sync() + errs := testGroupSyncer.Sync() if len(errs) != 1 { t.Errorf("unexpected sync error: %v", errs) @@ -333,6 +330,7 @@ func newTestSyncer() (*LDAPGroupSyncer, *testclient.Fake) { Host: newTestHost(), Out: ioutil.Discard, Err: ioutil.Discard, + MutationOutputOptions: mutation.NewFakeOptions(), }, tc } diff --git a/pkg/cmd/admin/project/new_project.go b/pkg/cmd/admin/project/new_project.go index 02ba1a2ca325..b1cbf05bb0f6 100644 --- a/pkg/cmd/admin/project/new_project.go +++ b/pkg/cmd/admin/project/new_project.go @@ -15,6 +15,7 @@ import ( "github.com/openshift/origin/pkg/cmd/admin/policy" "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/mutation" projectapi "github.com/openshift/origin/pkg/project/api" ) @@ -30,6 +31,9 @@ type NewProjectOptions struct { AdminRole string AdminUser string + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } const newProjectLong = ` @@ -52,6 +56,8 @@ func NewCmdNewProject(name, fullName string, f *clientcmd.Factory, out io.Writer kcmdutil.CheckErr(kcmdutil.UsageError(cmd, err.Error())) } + options.MutationOutputOptions = mutation.NewMutationOutputOptions(f.Factory, cmd, out) + var err error if options.Client, _, err = f.Clients(); err != nil { kcmdutil.CheckErr(err) @@ -72,6 +78,7 @@ func NewCmdNewProject(name, fullName string, f *clientcmd.Factory, out io.Writer cmd.Flags().StringVar(&options.DisplayName, "display-name", "", "Project display name") cmd.Flags().StringVar(&options.Description, "description", "", "Project description") cmd.Flags().StringVar(&options.NodeSelector, "node-selector", "", "Restrict pods onto nodes matching given label selector. Format: '=, =...'. Specifying \"\" means any node, not default. If unspecified, cluster default node selector will be used.") + kcmdutil.AddOutputFlagsForMutation(cmd) return cmd } @@ -107,7 +114,9 @@ func (o *NewProjectOptions) Run(useNodeSelector bool) error { return err } - fmt.Printf("Created project %v\n", o.ProjectName) + if err := o.MutationOutputOptions.PrintSuccess(project, "created"); err != nil { + return err + } errs := []error{} if len(o.AdminUser) != 0 { diff --git a/pkg/cmd/cli/secrets/basicauth.go b/pkg/cmd/cli/secrets/basicauth.go index b19d9c47d326..3a84441a1619 100644 --- a/pkg/cmd/cli/secrets/basicauth.go +++ b/pkg/cmd/cli/secrets/basicauth.go @@ -14,6 +14,7 @@ import ( "k8s.io/kubernetes/pkg/util/term" cmdutil "github.com/openshift/origin/pkg/cmd/util" + "github.com/openshift/origin/pkg/cmd/util/mutation" ) const ( @@ -53,6 +54,9 @@ type CreateBasicAuthSecretOptions struct { Out io.Writer SecretsInterface client.SecretsInterface + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } // NewCmdCreateBasicAuthSecret implements the OpenShift cli secrets new-basicauth subcommand @@ -72,6 +76,8 @@ func NewCmdCreateBasicAuthSecret(name, fullName string, f *kcmdutil.Factory, rea kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } + o.MutationOutputOptions = mutation.NewMutationOutputOptions(f, c, o.Out) + if err := o.Validate(); err != nil { kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } @@ -116,7 +122,9 @@ func (o *CreateBasicAuthSecretOptions) CreateBasicAuthSecret() error { return err } - fmt.Fprintf(o.GetOut(), "secret/%s\n", secret.Name) + if err := o.MutationOutputOptions.PrintSuccess(secret, "created"); err != nil { + return err + } return nil } diff --git a/pkg/cmd/cli/secrets/dockercfg.go b/pkg/cmd/cli/secrets/dockercfg.go index 2de946935fdc..7487ee40451a 100644 --- a/pkg/cmd/cli/secrets/dockercfg.go +++ b/pkg/cmd/cli/secrets/dockercfg.go @@ -13,6 +13,7 @@ import ( "k8s.io/kubernetes/pkg/credentialprovider" kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "github.com/openshift/origin/pkg/cmd/util/mutation" "github.com/spf13/cobra" ) @@ -56,6 +57,9 @@ type CreateDockerConfigOptions struct { SecretsInterface client.SecretsInterface Out io.Writer + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } // NewCmdCreateDockerConfigSecret creates a command object for making a dockercfg secret @@ -72,6 +76,8 @@ func NewCmdCreateDockerConfigSecret(name, fullName string, f *kcmdutil.Factory, kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } + o.MutationOutputOptions = mutation.NewMutationOutputOptions(f, c, o.Out) + if err := o.Validate(); err != nil { kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } @@ -110,8 +116,9 @@ func (o CreateDockerConfigOptions) CreateDockerSecret() error { return err } - fmt.Fprintf(o.GetOut(), "secret/%s\n", secret.Name) - + if err := o.MutationOutputOptions.PrintSuccess(secret, "created"); err != nil { + return err + } return nil } diff --git a/pkg/cmd/cli/secrets/new.go b/pkg/cmd/cli/secrets/new.go index 85569c295fa3..9766ed04bf64 100644 --- a/pkg/cmd/cli/secrets/new.go +++ b/pkg/cmd/cli/secrets/new.go @@ -15,6 +15,7 @@ import ( kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "github.com/openshift/origin/pkg/cmd/util/clientcmd" + "github.com/openshift/origin/pkg/cmd/util/mutation" "github.com/spf13/cobra" ) @@ -67,6 +68,9 @@ type CreateSecretOptions struct { Quiet bool AllowUnknownTypes bool + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } func NewCmdCreateSecret(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command { @@ -83,6 +87,8 @@ func NewCmdCreateSecret(name, fullName string, f *clientcmd.Factory, out io.Writ kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } + options.MutationOutputOptions = mutation.NewMutationOutputOptions(f.Factory, c, options.Out) + if err := options.Validate(); err != nil { kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } @@ -177,8 +183,11 @@ func (o *CreateSecretOptions) CreateSecret() (*kapi.Secret, error) { } persistedSecret, err := o.SecretsInterface.Create(secret) + if err == nil { - fmt.Fprintf(o.Out, "secret/%s\n", persistedSecret.Name) + if err := o.MutationOutputOptions.PrintSuccess(secret, "created"); err != nil { + return nil, err + } } return persistedSecret, err diff --git a/pkg/cmd/cli/secrets/sshauth.go b/pkg/cmd/cli/secrets/sshauth.go index 473eda1d7421..b609ab2f9344 100644 --- a/pkg/cmd/cli/secrets/sshauth.go +++ b/pkg/cmd/cli/secrets/sshauth.go @@ -10,6 +10,7 @@ import ( client "k8s.io/kubernetes/pkg/client/unversioned" kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "github.com/openshift/origin/pkg/cmd/util/mutation" "github.com/spf13/cobra" ) @@ -48,6 +49,9 @@ type CreateSSHAuthSecretOptions struct { Out io.Writer SecretsInterface client.SecretsInterface + + // MutationOutputOptions allows us to correctly output the mutations we make to objects in etcd + MutationOutputOptions mutation.MutationOutputOptions } // NewCmdCreateSSHAuthSecret implements the OpenShift cli secrets new-sshauth subcommand @@ -66,6 +70,8 @@ func NewCmdCreateSSHAuthSecret(name, fullName string, f *kcmdutil.Factory, out i kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } + o.MutationOutputOptions = mutation.NewMutationOutputOptions(f, c, o.Out) + if err := o.Validate(); err != nil { kcmdutil.CheckErr(kcmdutil.UsageError(c, err.Error())) } @@ -109,7 +115,9 @@ func (o *CreateSSHAuthSecretOptions) CreateSSHAuthSecret() error { return err } - fmt.Fprintf(o.GetOut(), "secret/%s\n", secret.Name) + if err := o.MutationOutputOptions.PrintSuccess(secret, "created"); err != nil { + return err + } return nil } diff --git a/pkg/cmd/util/mutation/mutate.go b/pkg/cmd/util/mutation/mutate.go new file mode 100644 index 000000000000..7087afe860e4 --- /dev/null +++ b/pkg/cmd/util/mutation/mutate.go @@ -0,0 +1,70 @@ +package mutation + +import ( + "io" + "io/ioutil" + + "github.com/spf13/cobra" + + "k8s.io/kubernetes/pkg/api/meta" + kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "k8s.io/kubernetes/pkg/kubectl/resource" + "k8s.io/kubernetes/pkg/runtime" +) + +func NewMutationOutputOptions(factory *kcmdutil.Factory, cmd *cobra.Command, out io.Writer) MutationOutputOptions { + mapper, typer := factory.Object() + if out == nil { + out = ioutil.Discard + } + + return &mutationOutputOptions{ + mapper: mapper, + resourceMapper: resource.Mapper{ObjectTyper: typer, RESTMapper: mapper, ClientMapper: factory.ClientMapperForCommand()}, + shortOutput: kcmdutil.GetFlagString(cmd, "output") == "name", + out: out, + } +} + +// MutationOutputOptions holds all of the information necessary to correctly output the reslult of mutating +// an object in etcd. +type MutationOutputOptions interface { + PrintSuccess(object runtime.Object, operation string) error + PrintSuccessForDescription(resource, name, operation string) +} + +type mutationOutputOptions struct { + mapper meta.RESTMapper + resourceMapper resource.Mapper + shortOutput bool + out io.Writer +} + +// PrintSuccess wraps around kcmdutil.PrintSuccess +func (o *mutationOutputOptions) PrintSuccess(object runtime.Object, operation string) error { + info, err := o.resourceMapper.InfoForObject(object) + if err != nil { + return err + } + kcmdutil.PrintSuccess(o.mapper, o.shortOutput, o.out, info.Mapping.Resource, info.Name, operation) + return nil +} + +// PrintSuccessForDescription allows for success printing when no object is present +func (o *mutationOutputOptions) PrintSuccessForDescription(resource, name, operation string) { + kcmdutil.PrintSuccess(o.mapper, o.shortOutput, o.out, resource, name, operation) +} + +// NewFakeOptions returns a fake set of options for use in testing +func NewFakeOptions() MutationOutputOptions { + return &fakeOutputOptions{} +} + +// fakeOutputOptions can be used for testing +type fakeOutputOptions struct{} + +func (o *fakeOutputOptions) PrintSuccess(object runtime.Object, operation string) error { + return nil +} + +func (o *fakeOutputOptions) PrintSuccessForDescription(resource, name, operation string) {} diff --git a/test/integration/groups_test.go b/test/integration/groups_test.go index 7e637bb7c2a3..b5db091176ce 100644 --- a/test/integration/groups_test.go +++ b/test/integration/groups_test.go @@ -8,6 +8,7 @@ import ( authorizationapi "github.com/openshift/origin/pkg/authorization/api" groupscmd "github.com/openshift/origin/pkg/cmd/admin/groups" + "github.com/openshift/origin/pkg/cmd/util/mutation" projectapi "github.com/openshift/origin/pkg/project/api" userapi "github.com/openshift/origin/pkg/user/api" uservalidation "github.com/openshift/origin/pkg/user/api/validation" @@ -182,7 +183,12 @@ func TestGroupCommands(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - newGroup := &groupscmd.NewGroupOptions{clusterAdminClient.Groups(), "group1", []string{"first", "second", "third", "first"}} + newGroup := &groupscmd.NewGroupOptions{ + GroupClient: clusterAdminClient.Groups(), + Group: "group1", + Users: []string{"first", "second", "third", "first"}, + MutationOutputOptions: mutation.NewFakeOptions(), + } if err := newGroup.AddGroup(); err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/test/integration/login_test.go b/test/integration/login_test.go index 19ff0c9e5426..e2ea4f67226b 100644 --- a/test/integration/login_test.go +++ b/test/integration/login_test.go @@ -17,6 +17,7 @@ import ( "github.com/openshift/origin/pkg/cmd/cli/cmd" "github.com/openshift/origin/pkg/cmd/cli/config" "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" + "github.com/openshift/origin/pkg/cmd/util/mutation" "github.com/openshift/origin/pkg/user/api" testutil "github.com/openshift/origin/test/util" testserver "github.com/openshift/origin/test/util/server" @@ -61,10 +62,11 @@ func TestLogin(t *testing.T) { } newProjectOptions := &newproject.NewProjectOptions{ - Client: clusterAdminClient, - ProjectName: project, - AdminRole: bootstrappolicy.AdminRoleName, - AdminUser: username, + Client: clusterAdminClient, + ProjectName: project, + AdminRole: bootstrappolicy.AdminRoleName, + AdminUser: username, + MutationOutputOptions: mutation.NewFakeOptions(), } if err := newProjectOptions.Run(false); err != nil { t.Fatalf("unexpected error, a project is required to continue: %v", err) diff --git a/test/util/server/server.go b/test/util/server/server.go index 923f1d409d68..48c0ac751b54 100644 --- a/test/util/server/server.go +++ b/test/util/server/server.go @@ -11,6 +11,7 @@ import ( "github.com/golang/glog" "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" + "github.com/openshift/origin/pkg/cmd/util/mutation" kapi "k8s.io/kubernetes/pkg/api" kclient "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/util/wait" @@ -424,6 +425,8 @@ func CreateNewProject(clusterAdminClient *client.Client, clientConfig kclient.Co ProjectName: projectName, AdminRole: bootstrappolicy.AdminRoleName, AdminUser: adminUser, + + MutationOutputOptions: mutation.NewFakeOptions(), } if err := newProjectOptions.Run(false); err != nil {