Skip to content

Commit 0c52d07

Browse files
smarterclaytonsmarterclayton
committed
Remove the use of checkErr in our codebase
Replace with proper error returning.
1 parent 269b892 commit 0c52d07

File tree

13 files changed

+786
-660
lines changed

13 files changed

+786
-660
lines changed

Diff for: ‎pkg/cmd/cli/cmd/buildlogs.go

+27-14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66

77
"github.com/spf13/cobra"
88

9+
cmdutil "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/util"
10+
911
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
1012
)
1113

@@ -25,23 +27,34 @@ func NewCmdBuildLogs(fullName string, f *clientcmd.Factory, out io.Writer) *cobr
2527
Short: "Show container logs from the build container",
2628
Long: fmt.Sprintf(buildLogsLongDesc, fullName),
2729
Run: func(cmd *cobra.Command, args []string) {
28-
if len(args) != 1 {
29-
usageError(cmd, "<build> is a required argument")
30-
}
30+
err := RunBuildLogs(f, out, cmd, args)
31+
cmdutil.CheckErr(err)
32+
},
33+
}
34+
return cmd
35+
}
3136

32-
namespace, err := f.DefaultNamespace()
33-
checkErr(err)
37+
func RunBuildLogs(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []string) error {
38+
if len(args) != 1 {
39+
return cmdutil.UsageError(cmd, "<build> is a required argument")
40+
}
3441

35-
c, _, err := f.Clients()
36-
checkErr(err)
42+
namespace, err := f.DefaultNamespace()
43+
if err != nil {
44+
return err
45+
}
3746

38-
readCloser, err := c.BuildLogs(namespace).Get(args[0]).Stream()
39-
checkErr(err)
40-
defer readCloser.Close()
47+
c, _, err := f.Clients()
48+
if err != nil {
49+
return err
50+
}
4151

42-
_, err = io.Copy(out, readCloser)
43-
checkErr(err)
44-
},
52+
readCloser, err := c.BuildLogs(namespace).Get(args[0]).Stream()
53+
if err != nil {
54+
return err
4555
}
46-
return cmd
56+
defer readCloser.Close()
57+
58+
_, err = io.Copy(out, readCloser)
59+
return err
4760
}

Diff for: ‎pkg/cmd/cli/cmd/cancelbuild.go

+73-57
Original file line numberDiff line numberDiff line change
@@ -33,78 +33,94 @@ Examples:
3333
// To cancel a build its name has to be specified, and two options
3434
// are available: displaying logs and restarting.
3535
func NewCmdCancelBuild(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
36-
3736
cmd := &cobra.Command{
3837
Use: "cancel-build <build>",
3938
Short: "Cancel a pending or running build.",
4039
Long: fmt.Sprintf(cancelBuildLongDesc, fullName),
4140
Run: func(cmd *cobra.Command, args []string) {
41+
err := RunCancelBuild(f, out, cmd, args)
42+
cmdutil.CheckErr(err)
43+
},
44+
}
4245

43-
if len(args) == 0 || len(args[0]) == 0 {
44-
usageError(cmd, "You must specify the name of a build to cancel.")
45-
}
46+
cmd.Flags().Bool("dump-logs", false, "Specify if the build logs for the cancelled build should be shown.")
47+
cmd.Flags().Bool("restart", false, "Specify if a new build should be created after the current build is cancelled.")
48+
return cmd
49+
}
4650

47-
buildName := args[0]
48-
namespace, err := f.DefaultNamespace()
49-
checkErr(err)
51+
func RunCancelBuild(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []string) error {
52+
if len(args) == 0 || len(args[0]) == 0 {
53+
return cmdutil.UsageError(cmd, "You must specify the name of a build to cancel.")
54+
}
5055

51-
client, _, err := f.Clients()
52-
checkErr(err)
53-
buildClient := client.Builds(namespace)
54-
build, err := buildClient.Get(buildName)
55-
checkErr(err)
56+
buildName := args[0]
57+
namespace, err := f.DefaultNamespace()
58+
if err != nil {
59+
return err
60+
}
5661

57-
if !isBuildCancellable(build) {
58-
return
59-
}
62+
client, _, err := f.Clients()
63+
if err != nil {
64+
return err
65+
}
66+
buildClient := client.Builds(namespace)
67+
build, err := buildClient.Get(buildName)
68+
if err != nil {
69+
return err
70+
}
6071

61-
// Print build logs before cancelling build.
62-
if cmdutil.GetFlagBool(cmd, "dump-logs") {
63-
// in order to dump logs, you must have a pod assigned to the build. Since build pod creation is asynchronous, it is possible to cancel a build without a pod being assigned.
64-
if build.Status != buildapi.BuildStatusRunning {
65-
glog.V(2).Infof("Build %v has not yet generated any logs.", buildName)
66-
67-
} else {
68-
response, err := client.BuildLogs(namespace).Get(buildName).Do().Raw()
69-
if err != nil {
70-
glog.Errorf("Could not fetch build logs for %s: %v", buildName, err)
71-
} else {
72-
glog.V(2).Infof("Build logs for %s:\n%v", buildName, string(response))
73-
}
74-
}
75-
}
72+
if !isBuildCancellable(build) {
73+
return nil
74+
}
7675

77-
// Mark build to be cancelled.
78-
for {
79-
build.Cancelled = true
80-
if _, err = buildClient.Update(build); err != nil && errors.IsConflict(err) {
81-
build, err = buildClient.Get(buildName)
82-
checkErr(err)
83-
continue
84-
}
85-
checkErr(err)
86-
break
87-
}
88-
glog.V(2).Infof("Build %s was cancelled.", buildName)
89-
90-
// Create a new build with the same configuration.
91-
if cmdutil.GetFlagBool(cmd, "restart") {
92-
request := &buildapi.BuildRequest{
93-
ObjectMeta: kapi.ObjectMeta{Name: build.Name},
94-
}
95-
newBuild, err := client.Builds(namespace).Clone(request)
96-
checkErr(err)
97-
glog.V(2).Infof("Restarted build %s.", buildName)
98-
fmt.Fprintf(out, "%s\n", newBuild.Name)
76+
// Print build logs before cancelling build.
77+
if cmdutil.GetFlagBool(cmd, "dump-logs") {
78+
// in order to dump logs, you must have a pod assigned to the build. Since build pod creation is asynchronous, it is possible to cancel a build without a pod being assigned.
79+
if build.Status != buildapi.BuildStatusRunning {
80+
glog.V(2).Infof("Build %v has not yet generated any logs.", buildName)
81+
82+
} else {
83+
response, err := client.BuildLogs(namespace).Get(buildName).Do().Raw()
84+
if err != nil {
85+
glog.Errorf("Could not fetch build logs for %s: %v", buildName, err)
9986
} else {
100-
fmt.Fprintf(out, "%s\n", build.Name)
87+
glog.V(2).Infof("Build logs for %s:\n%v", buildName, string(response))
10188
}
102-
},
89+
}
10390
}
10491

105-
cmd.Flags().Bool("dump-logs", false, "Specify if the build logs for the cancelled build should be shown.")
106-
cmd.Flags().Bool("restart", false, "Specify if a new build should be created after the current build is cancelled.")
107-
return cmd
92+
// Mark build to be cancelled.
93+
for {
94+
build.Cancelled = true
95+
if _, err = buildClient.Update(build); err != nil && errors.IsConflict(err) {
96+
build, err = buildClient.Get(buildName)
97+
if err != nil {
98+
return err
99+
}
100+
continue
101+
}
102+
if err != nil {
103+
return err
104+
}
105+
break
106+
}
107+
glog.V(2).Infof("Build %s was cancelled.", buildName)
108+
109+
// Create a new build with the same configuration.
110+
if cmdutil.GetFlagBool(cmd, "restart") {
111+
request := &buildapi.BuildRequest{
112+
ObjectMeta: kapi.ObjectMeta{Name: build.Name},
113+
}
114+
newBuild, err := client.Builds(namespace).Clone(request)
115+
if err != nil {
116+
return err
117+
}
118+
glog.V(2).Infof("Restarted build %s.", buildName)
119+
fmt.Fprintf(out, "%s\n", newBuild.Name)
120+
} else {
121+
fmt.Fprintf(out, "%s\n", build.Name)
122+
}
123+
return nil
108124
}
109125

110126
// isBuildCancellable checks if another cancellation event was triggered, and if the build status is correct.

Diff for: ‎pkg/cmd/cli/cmd/helpers.go

-20
This file was deleted.

Diff for: ‎pkg/cmd/cli/cmd/login.go

+33-22
Original file line numberDiff line numberDiff line change
@@ -25,34 +25,19 @@ prompt for user input as needed.
2525
`
2626

2727
func NewCmdLogin(f *osclientcmd.Factory, reader io.Reader, out io.Writer) *cobra.Command {
28-
options := &LoginOptions{}
28+
options := &LoginOptions{
29+
Reader: reader,
30+
Out: out,
31+
ClientConfig: f.OpenShiftClientConfig,
32+
}
2933

3034
cmds := &cobra.Command{
3135
Use: "login [--username=<username>] [--password=<password>] [--server=<server>] [--context=<context>] [--certificate-authority=<path>]",
3236
Short: "Logs in and save the configuration",
3337
Long: longDescription,
3438
Run: func(cmd *cobra.Command, args []string) {
35-
options.Reader = reader
36-
options.ClientConfig = f.OpenShiftClientConfig
37-
38-
if certFile := cmdutil.GetFlagString(cmd, "client-certificate"); len(certFile) > 0 {
39-
options.CertFile = certFile
40-
}
41-
if keyFile := cmdutil.GetFlagString(cmd, "client-key"); len(keyFile) > 0 {
42-
options.KeyFile = keyFile
43-
}
44-
45-
checkErr(options.GatherInfo())
46-
47-
forcePath := cmdutil.GetFlagString(cmd, config.OpenShiftConfigFlagName)
48-
options.PathToSaveConfig = forcePath
49-
50-
newFileCreated, err := options.SaveConfig()
51-
checkErr(err)
52-
53-
if newFileCreated {
54-
fmt.Println("Welcome to OpenShift! See 'osc help' for to get started.")
55-
}
39+
err := RunLogin(cmd, options)
40+
cmdutil.CheckErr(err)
5641
},
5742
}
5843

@@ -70,3 +55,29 @@ func NewCmdLogin(f *osclientcmd.Factory, reader io.Reader, out io.Writer) *cobra
7055

7156
return cmds
7257
}
58+
59+
func RunLogin(cmd *cobra.Command, options *LoginOptions) error {
60+
if certFile := cmdutil.GetFlagString(cmd, "client-certificate"); len(certFile) > 0 {
61+
options.CertFile = certFile
62+
}
63+
if keyFile := cmdutil.GetFlagString(cmd, "client-key"); len(keyFile) > 0 {
64+
options.KeyFile = keyFile
65+
}
66+
67+
if err := options.GatherInfo(); err != nil {
68+
return err
69+
}
70+
71+
forcePath := cmdutil.GetFlagString(cmd, config.OpenShiftConfigFlagName)
72+
options.PathToSaveConfig = forcePath
73+
74+
newFileCreated, err := options.SaveConfig()
75+
if err != nil {
76+
return err
77+
}
78+
79+
if newFileCreated {
80+
fmt.Fprintln(options.Out, "Welcome to OpenShift! See 'osc help' to get started.")
81+
}
82+
return nil
83+
}

Diff for: ‎pkg/cmd/cli/cmd/loginoptions.go

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ type LoginOptions struct {
3939
ClientConfig kclientcmd.ClientConfig
4040
Config *kclient.Config
4141
Reader io.Reader
42+
Out io.Writer
4243

4344
// cert data to be used when authenticating
4445
CertFile string

0 commit comments

Comments
 (0)