unified printing strategy

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")

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")

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
 }

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)

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) {

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

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
 
 }

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,21 +44,23 @@ 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
 	glog.V(1).Infof("Listing with %v", s.GroupLister)
 	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

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
 
 }