From a0835edf9a7b22511ff4add26242728fcacceb3c Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Fri, 24 Apr 2020 11:05:29 +0200 Subject: [PATCH] Move command pre-checks into dedicated packages Some commands are not necessarily required to be run `krel gcbmgr`. We now move the checks into the git and gcp package (new) to have a more clear structure which package requires which CLI dependency. Signed-off-by: Sascha Grunert --- BUILD.bazel | 3 +-- cmd/krel/cmd/BUILD.bazel | 1 + cmd/krel/cmd/gcbmgr.go | 32 ++++------------------- pkg/gcp/BUILD.bazel | 30 +++++++++++++++++++++ pkg/gcp/auth/BUILD.bazel | 1 + pkg/gcp/auth/auth.go | 3 ++- pkg/gcp/build/BUILD.bazel | 1 + pkg/gcp/build/build.go | 7 ++--- pkg/gcp/gcp.go | 46 +++++++++++++++++++++++++++++++++ pkg/git/git.go | 33 ++++++++++------------- pkg/git/git_integration_test.go | 11 +++++++- 11 files changed, 114 insertions(+), 54 deletions(-) create mode 100644 pkg/gcp/BUILD.bazel create mode 100644 pkg/gcp/gcp.go diff --git a/BUILD.bazel b/BUILD.bazel index 86071a39e11..6c2f89a3b85 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -35,8 +35,7 @@ filegroup( "//cmd/release-notes:all-srcs", "//lib:all-srcs", "//pkg/command:all-srcs", - "//pkg/gcp/auth:all-srcs", - "//pkg/gcp/build:all-srcs", + "//pkg/gcp:all-srcs", "//pkg/git:all-srcs", "//pkg/github:all-srcs", "//pkg/http:all-srcs", diff --git a/cmd/krel/cmd/BUILD.bazel b/cmd/krel/cmd/BUILD.bazel index cbe2f59f28a..67b6575388c 100644 --- a/cmd/krel/cmd/BUILD.bazel +++ b/cmd/krel/cmd/BUILD.bazel @@ -16,6 +16,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/command:go_default_library", + "//pkg/gcp:go_default_library", "//pkg/gcp/auth:go_default_library", "//pkg/gcp/build:go_default_library", "//pkg/git:go_default_library", diff --git a/cmd/krel/cmd/gcbmgr.go b/cmd/krel/cmd/gcbmgr.go index 3922734fd10..a8e58a3ec67 100644 --- a/cmd/krel/cmd/gcbmgr.go +++ b/cmd/krel/cmd/gcbmgr.go @@ -26,7 +26,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "k8s.io/release/pkg/command" + "k8s.io/release/pkg/gcp" "k8s.io/release/pkg/gcp/auth" "k8s.io/release/pkg/gcp/build" "k8s.io/release/pkg/git" @@ -64,18 +64,6 @@ type Version interface { var ( gcbmgrOpts = &GcbmgrOptions{} buildOpts = &build.Options{} - - requiredPackages = []string{ - // "bsdmainutils", - } - - // TODO: Do we really need this if we use the Google Cloud SDK instead? - requiredCommands = []string{ - "gcloud", - "git", - "gsutil", - "jq", - } ) // gcbmgrCmd is a krel subcommand which invokes RunGcbmgr() @@ -168,24 +156,14 @@ func init() { // RunGcbmgr is the function invoked by 'krel gcbmgr', responsible for // submitting release jobs to GCB func RunGcbmgr(opts *GcbmgrOptions) error { - logrus.Info("Checking for required packages") - ok, err := util.PackagesAvailable(requiredPackages...) - if err != nil { - return errors.Wrap(err, "unable to verify if packages are available") - } - if !ok { - return errors.New("packages required to run gcbmgr are not present") - } - - logrus.Info("Checking for required commands") - if cmdAvailable := command.Available(requiredCommands...); !cmdAvailable { - return errors.New("binaries required to run gcbmgr are not present") - } - toolOrg := release.GetToolOrg() toolRepo := release.GetToolRepo() toolBranch := release.GetToolBranch() + if err := gcp.PreCheck(); err != nil { + return errors.Wrap(err, "pre-checking for GCP package") + } + if err := opts.Repo.Open(); err != nil { return errors.Wrap(err, "open release repo") } diff --git a/pkg/gcp/BUILD.bazel b/pkg/gcp/BUILD.bazel new file mode 100644 index 00000000000..a5bfcf5692c --- /dev/null +++ b/pkg/gcp/BUILD.bazel @@ -0,0 +1,30 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["gcp.go"], + importpath = "k8s.io/release/pkg/gcp", + visibility = ["//visibility:public"], + deps = [ + "//pkg/command:go_default_library", + "@com_github_pkg_errors//:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//pkg/gcp/auth:all-srcs", + "//pkg/gcp/build:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/gcp/auth/BUILD.bazel b/pkg/gcp/auth/BUILD.bazel index a42f73d5a7c..b6bb178e3ef 100644 --- a/pkg/gcp/auth/BUILD.bazel +++ b/pkg/gcp/auth/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/command:go_default_library", + "//pkg/gcp:go_default_library", "@com_github_pkg_errors//:go_default_library", ], ) diff --git a/pkg/gcp/auth/auth.go b/pkg/gcp/auth/auth.go index b4f9ef4bfc3..cc5bfc20861 100644 --- a/pkg/gcp/auth/auth.go +++ b/pkg/gcp/auth/auth.go @@ -22,11 +22,12 @@ import ( "github.com/pkg/errors" "k8s.io/release/pkg/command" + "k8s.io/release/pkg/gcp" ) func GetCurrentGCPUser() (string, error) { authListStatus, authListErr := command.New( - "gcloud", + gcp.GCloudExecutable, "auth", "list", "--filter=status:ACTIVE", diff --git a/pkg/gcp/build/BUILD.bazel b/pkg/gcp/build/BUILD.bazel index 1ceba2995c4..46c97d9d201 100644 --- a/pkg/gcp/build/BUILD.bazel +++ b/pkg/gcp/build/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/command:go_default_library", + "//pkg/gcp:go_default_library", "//pkg/release:go_default_library", "@com_github_google_uuid//:go_default_library", "@com_github_olekukonko_tablewriter//:go_default_library", diff --git a/pkg/gcp/build/build.go b/pkg/gcp/build/build.go index 6f41a5a455a..bc38d2e7ed7 100644 --- a/pkg/gcp/build/build.go +++ b/pkg/gcp/build/build.go @@ -35,6 +35,7 @@ import ( "google.golang.org/api/option" "k8s.io/release/pkg/command" + "k8s.io/release/pkg/gcp" "k8s.io/release/pkg/release" "sigs.k8s.io/yaml" ) @@ -138,7 +139,7 @@ func (o *Options) uploadBuildDir(targetBucket string) (string, error) { logrus.Infof("Creating source tarball at %s...", name) tarCmdErr := command.Execute( - "tar", + gcp.TarExecutable, "--exclude", ".git", "-czf", @@ -153,7 +154,7 @@ func (o *Options) uploadBuildDir(targetBucket string) (string, error) { uploaded := fmt.Sprintf("%s/%s.tgz", targetBucket, u.String()) logrus.Infof("Uploading %s to %s...", name, uploaded) cpErr := command.Execute( - "gsutil", + gcp.GSUtilExecutable, "cp", name, uploaded, @@ -235,7 +236,7 @@ func RunSingleJob(o *Options, jobName, uploaded, version string, subs map[string args = append(args, diskSizeArg) } - cmd := command.New("gcloud", args...) + cmd := command.New(gcp.GCloudExecutable, args...) if o.LogDir != "" { p := path.Join(o.LogDir, strings.ReplaceAll(jobName, "/", "-")+".log") diff --git a/pkg/gcp/gcp.go b/pkg/gcp/gcp.go new file mode 100644 index 00000000000..1f0a5111411 --- /dev/null +++ b/pkg/gcp/gcp.go @@ -0,0 +1,46 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package gcp + +import ( + "github.com/pkg/errors" + "k8s.io/release/pkg/command" +) + +const ( + GCloudExecutable = "tar" + GSUtilExecutable = "gsutil" + TarExecutable = "gcloud" +) + +// PreCheck checks if all requirements are fulfilled to run this package and +// all sub-packages +func PreCheck() error { + for _, e := range []string{ + GCloudExecutable, + GSUtilExecutable, + TarExecutable, + } { + if !command.Available(e) { + return errors.Errorf( + "%s executable is not available in $PATH", e, + ) + } + } + + return nil +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 15d2effd6cd..dbb83877b10 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -214,11 +214,6 @@ func CloneOrOpenGitHubRepo(repoPath, owner, repo string, useSSH bool) (*Repo, er // The function returns the repository if cloning or updating of the repository // was successful, otherwise an error. func CloneOrOpenRepo(repoPath, repoURL string, useSSH bool) (*Repo, error) { - // We still need the plain git executable for some methods - if !command.Available(gitExecutable) { - return nil, errors.New("git is needed to support all repository features") - } - logrus.Debugf("Using repository url %q", repoURL) targetDir := "" if repoPath != "" { @@ -256,47 +251,45 @@ func CloneOrOpenRepo(repoPath, repoURL string, useSSH bool) (*Repo, error) { // updateRepo tries to open the provided repoPath and fetches the latest // changes from the configured remote location func updateRepo(repoPath string) (*Repo, error) { - r, err := git.PlainOpen(repoPath) + r, err := OpenRepo(repoPath) if err != nil { - return nil, errors.Wrap(err, "unable to open repo") + return nil, err } // Update the repo if err := command.NewWithWorkDir( - repoPath, gitExecutable, "pull", "--rebase", + r.Dir(), gitExecutable, "pull", "--rebase", ).RunSilentSuccess(); err != nil { return nil, errors.Wrap(err, "unable to pull from remote") } - worktree, err := r.Worktree() - if err != nil { - return nil, errors.Wrap(err, "unable to get repository worktree") - } - return &Repo{ - inner: r, - worktree: worktree, - dir: repoPath, - }, nil + return r, nil } // OpenRepo tries to open the provided repoPath func OpenRepo(repoPath string) (*Repo, error) { + if !command.Available(gitExecutable) { + return nil, errors.Errorf( + "%s executable is not available in $PATH", gitExecutable, + ) + } + r, err := git.PlainOpenWithOptions( repoPath, &git.PlainOpenOptions{DetectDotGit: true}, ) if err != nil { - return nil, err + return nil, errors.Wrap(err, "opening repo") } worktree, err := r.Worktree() if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting repository worktree") } return &Repo{ inner: r, worktree: worktree, - dir: repoPath, + dir: worktree.Filesystem.Root(), }, nil } diff --git a/pkg/git/git_integration_test.go b/pkg/git/git_integration_test.go index 652a63393d6..6174d9b1964 100644 --- a/pkg/git/git_integration_test.go +++ b/pkg/git/git_integration_test.go @@ -750,8 +750,17 @@ func TestOpenRepoSuccess(t *testing.T) { require.Equal(t, testRepo.sut.Dir(), repo.Dir()) } +func TestOpenRepoSuccessSearchGitDot(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + repo, err := git.OpenRepo(filepath.Join(testRepo.sut.Dir(), "not-existing")) + require.Nil(t, err) + require.Equal(t, testRepo.sut.Dir(), repo.Dir()) +} + func TestOpenRepoFailure(t *testing.T) { - repo, err := git.OpenRepo("invalid") + repo, err := git.OpenRepo("/invalid") require.NotNil(t, err) require.Nil(t, repo) }