From 134896615f0edff04d5f51ffe3d74e98bb8e86f0 Mon Sep 17 00:00:00 2001 From: Daniel Hiltgen Date: Thu, 1 Apr 2021 13:49:22 -0700 Subject: [PATCH] Enhance version command This improves the version command to report both client and builder version information. --- integration/common/basesuites.go | 8 +-- integration/common/runners.go | 35 ++++++++++-- integration/suites/configmap_test.go | 16 +++--- integration/suites/default_test.go | 13 +++++ integration/suites/localregistry_test.go | 29 +++++++--- integration/suites/parallel_default_test.go | 2 +- integration/suites/rootless_test.go | 2 +- pkg/cmd/root.go | 2 +- pkg/cmd/version.go | 54 +++++++++++++++++-- pkg/driver/driver.go | 1 + pkg/driver/kubernetes/driver.go | 54 +++++++++++++++++++ .../kubernetes/podchooser/podchooser.go | 9 +++- 12 files changed, 192 insertions(+), 33 deletions(-) diff --git a/integration/common/basesuites.go b/integration/common/basesuites.go index 2865781d..40d19cee 100644 --- a/integration/common/basesuites.go +++ b/integration/common/basesuites.go @@ -36,7 +36,7 @@ func (s *BaseSuite) SetupSuite() { }, s.CreateFlags..., ) - err := RunBuildkit("create", args) + err := RunBuildkit("create", args, RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder create failed", s.Name) } @@ -49,7 +49,7 @@ func (s *BaseSuite) TearDownSuite() { logrus.Infof("%s: Removing builder", s.Name) err := RunBuildkit("rm", []string{ s.Name, - }) + }, RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder rm failed", s.Name) configMapClient := s.ClientSet.CoreV1().ConfigMaps(s.Namespace) _, err = configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) @@ -76,7 +76,7 @@ func (s *BaseSuite) TestSimpleBuild() { "--tag", imageName, dir, ) - err = RunBuild(args) + err = RunBuild(args, RunBuildStreams{}) if isRootlessCreate(s.CreateFlags) { require.Error(s.T(), err) require.Contains(s.T(), err.Error(), "please specify") @@ -116,7 +116,7 @@ func (s *BaseSuite) TestLocalOutputTarBuild() { fmt.Sprintf("--output=type=tar,dest=%s", path.Join(dir, "out.tar")), dir, ) - err = RunBuild(args) + err = RunBuild(args, RunBuildStreams{}) require.NoError(s.T(), err, "build failed") // TODO - consider inspecting the out.tar for validity... } diff --git a/integration/common/runners.go b/integration/common/runners.go index 23afa145..18ed15c6 100644 --- a/integration/common/runners.go +++ b/integration/common/runners.go @@ -3,6 +3,7 @@ package common import ( + "io" "os" "github.com/sirupsen/logrus" @@ -15,23 +16,40 @@ import ( _ "github.com/vmware-tanzu/buildkit-cli-for-kubectl/pkg/driver/kubernetes" ) -func RunBuild(args []string) error { +// RunBuildStreams can override in/out/err streams if the output needs to be evaluated +// if unset, stdin/stdout/stderr will be used +type RunBuildStreams struct { + In io.Reader + Out io.Writer + Err io.Writer +} + +func RunBuild(args []string, streams RunBuildStreams) error { flags := pflag.NewFlagSet("kubectl-build", pflag.ExitOnError) pflag.CommandLine = flags finalArgs := append( []string{"--kubeconfig", os.Getenv("TEST_KUBECONFIG")}, args..., ) + if streams.In == nil { + streams.In = os.Stdin + } + if streams.Out == nil { + streams.Out = os.Stdout + } + if streams.Err == nil { + streams.Err = os.Stderr + } // TODO do we want to capture the output someplace else? - root := commands.NewRootBuildCmd(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}) + root := commands.NewRootBuildCmd(genericclioptions.IOStreams{In: streams.In, Out: streams.Out, ErrOut: streams.Err}) root.SetArgs(finalArgs) logrus.Infof("Build: %v", finalArgs) return root.Execute() } -func RunBuildkit(command string, args []string) error { +func RunBuildkit(command string, args []string, streams RunBuildStreams) error { flags := pflag.NewFlagSet("kubectl-buildkit", pflag.ExitOnError) pflag.CommandLine = flags finalArgs := append( @@ -39,8 +57,17 @@ func RunBuildkit(command string, args []string) error { args..., ) logrus.Infof("CMD: %v", finalArgs) + if streams.In == nil { + streams.In = os.Stdin + } + if streams.Out == nil { + streams.Out = os.Stdout + } + if streams.Err == nil { + streams.Err = os.Stderr + } - root := commands.NewRootCmd(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}) + root := commands.NewRootCmd(genericclioptions.IOStreams{In: streams.In, Out: streams.Out, ErrOut: streams.Err}) root.SetArgs(finalArgs) return root.Execute() diff --git a/integration/suites/configmap_test.go b/integration/suites/configmap_test.go index a757bded..dd99e31e 100644 --- a/integration/suites/configmap_test.go +++ b/integration/suites/configmap_test.go @@ -64,7 +64,7 @@ func (s *configMapSuite) TestDefaultCreate() { }, s.CreateFlags..., ) - err := common.RunBuildkit("create", args) + err := common.RunBuildkit("create", args, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder create failed", s.Name) cfg, err := s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.NoError(s.T(), err, "%s: fetch configmap failed", s.Name) @@ -78,7 +78,7 @@ func (s *configMapSuite) TestDefaultCreate() { logrus.Infof("%s: Removing builder", s.Name) err = common.RunBuildkit("rm", []string{ s.Name, - }) + }, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder rm failed", s.Name) _, err = s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.Error(s.T(), err, "config map wasn't cleaned up") @@ -98,7 +98,7 @@ func (s *configMapSuite) TestPreExistingConfigDefaultCreate() { }, s.CreateFlags..., ) - err = common.RunBuildkit("create", args) + err = common.RunBuildkit("create", args, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder create failed", s.Name) cfg, err := s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.NoError(s.T(), err, "%s: fetch configmap failed", s.Name) @@ -112,7 +112,7 @@ func (s *configMapSuite) TestPreExistingConfigDefaultCreate() { logrus.Infof("%s: Removing builder", s.Name) err = common.RunBuildkit("rm", []string{ s.Name, - }) + }, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder rm failed", s.Name) _, err = s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) // TODO if we preserve pre-existing configmaps this will need to be refined. @@ -137,7 +137,7 @@ func (s *configMapSuite) TestCustomCreate() { }, s.CreateFlags..., ) - err = common.RunBuildkit("create", args) + err = common.RunBuildkit("create", args, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder create failed", s.Name) cfg, err := s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.NoError(s.T(), err, "%s: fetch configmap failed", s.Name) @@ -152,7 +152,7 @@ func (s *configMapSuite) TestCustomCreate() { logrus.Infof("%s: Removing builder", s.Name) err = common.RunBuildkit("rm", []string{ s.Name, - }) + }, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder rm failed", s.Name) _, err = s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.Error(s.T(), err, "config map wasn't cleaned up") @@ -179,7 +179,7 @@ func (s *configMapSuite) TestPreExistingWithCustomCreate() { }, s.CreateFlags..., ) - err = common.RunBuildkit("create", args) + err = common.RunBuildkit("create", args, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder create failed", s.Name) cfg, err := s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.NoError(s.T(), err, "%s: fetch configmap failed", s.Name) @@ -195,7 +195,7 @@ func (s *configMapSuite) TestPreExistingWithCustomCreate() { logrus.Infof("%s: Removing builder", s.Name) err = common.RunBuildkit("rm", []string{ s.Name, - }) + }, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder rm failed", s.Name) _, err = s.configMapClient.Get(context.Background(), s.Name, metav1.GetOptions{}) require.Error(s.T(), err, "config map wasn't cleaned up") diff --git a/integration/suites/default_test.go b/integration/suites/default_test.go index 3825a439..95f1ffaa 100644 --- a/integration/suites/default_test.go +++ b/integration/suites/default_test.go @@ -3,8 +3,11 @@ package suites import ( + "bytes" + "strings" "testing" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/vmware-tanzu/buildkit-cli-for-kubectl/integration/common" @@ -12,6 +15,16 @@ import ( type DefaultSuite struct{ common.BaseSuite } +func (s *DefaultSuite) TestVersion() { + buf := &bytes.Buffer{} + err := common.RunBuildkit("version", []string{}, common.RunBuildStreams{Out: buf}) + require.NoError(s.T(), err, "%s: builder version failed", s.Name) + lines := strings.Split(strings.TrimSpace(buf.String()), "\n") + require.Len(s.T(), lines, 2) + require.Contains(s.T(), lines[0], "Client:") + require.Contains(s.T(), lines[1], "buildkitd") +} + func TestDefaultSuite(t *testing.T) { common.Skipper(t) t.Parallel() diff --git a/integration/suites/localregistry_test.go b/integration/suites/localregistry_test.go index 8654bddc..1a9a1dc2 100644 --- a/integration/suites/localregistry_test.go +++ b/integration/suites/localregistry_test.go @@ -3,6 +3,7 @@ package suites import ( + "bytes" "context" "fmt" "path/filepath" @@ -236,7 +237,7 @@ func (s *localRegistrySuite) SetupSuite() { }, s.CreateFlags..., ) - err = common.RunBuildkit("create", args) + err = common.RunBuildkit("create", args, common.RunBuildStreams{}) require.NoError(s.T(), err, "%s: builder creation failed", s.Name) } @@ -267,7 +268,7 @@ func (s *localRegistrySuite) TearDownSuite() { logrus.Infof("%s: Removing builder", s.Name) err = common.RunBuildkit("rm", []string{ s.Name, - }) + }, common.RunBuildStreams{}) if err != nil { logrus.Warnf("failed to clean up builder %s", err) } @@ -288,7 +289,7 @@ func (s *localRegistrySuite) TestBuildWithPush() { "--tag", imageName, dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.NoError(s.T(), err, "build failed") // Note, we can't run the image we just built since it was pushed to the local registry, which isn't ~directly visible to the runtime } @@ -318,14 +319,14 @@ func (s *localRegistrySuite) TestBuildWithCacheScenarios() { "--cache-to", "type=registry,ref=" + cacheName, dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.NoError(s.T(), err, "cache-to only build failed") // Now do another build with cache-to and cache-from args = append(args, "--cache-from", "type=registry,ref="+cacheName, ) - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.NoError(s.T(), err, "cache-to/from build failed") // Do a build with inline caching @@ -337,7 +338,7 @@ func (s *localRegistrySuite) TestBuildWithCacheScenarios() { "--cache-from", "type=registry,ref=" + cacheName, dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.NoError(s.T(), err, "inline cache build failed") // Note, we can't run the image we just built since it was pushed to the local registry, which isn't ~directly visible to the runtime @@ -354,7 +355,7 @@ func (s *localRegistrySuite) TestBuildPushWithoutTag() { "--push", dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.Error(s.T(), err) require.Contains(s.T(), err.Error(), "tag is needed when pushing to registry") } @@ -402,12 +403,24 @@ func main() { "--platform", "linux/386,linux/amd64,linux/arm/v6,linux/arm/v7,linux/arm64,windows/amd64", dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.NoError(s.T(), err, "cross-compile multi-arch build failed") // TODO - need to poke at the resulting image to make sure it was actually correctly created... } +func (s *localRegistrySuite) TestVersion() { + buf := &bytes.Buffer{} + err := common.RunBuildkit("version", []string{ + "--builder", s.Name, + }, common.RunBuildStreams{Out: buf}) + require.NoError(s.T(), err, "%s: builder version failed", s.Name) + lines := strings.Split(strings.TrimSpace(buf.String()), "\n") + require.Len(s.T(), lines, 2) + require.Contains(s.T(), lines[0], "Client:") + require.Contains(s.T(), lines[1], "buildkitd") +} + func TestLocalRegistrySuite(t *testing.T) { common.Skipper(t) // TODO this testcase should be safe to run parallel, but I'm seeing failures in CI that look diff --git a/integration/suites/parallel_default_test.go b/integration/suites/parallel_default_test.go index 55c16ef0..faef4e1c 100644 --- a/integration/suites/parallel_default_test.go +++ b/integration/suites/parallel_default_test.go @@ -62,7 +62,7 @@ func (s *parallelDefaultSuite) TestParallelDefaultBuilds() { "--tag", imageName, dirs[i], } - err := common.RunBuild(args) + err := common.RunBuild(args, common.RunBuildStreams{}) if err != nil { errors[i] = err return diff --git a/integration/suites/rootless_test.go b/integration/suites/rootless_test.go index 18a45b5c..2e6b2471 100644 --- a/integration/suites/rootless_test.go +++ b/integration/suites/rootless_test.go @@ -34,7 +34,7 @@ func (s *rootlessSuite) TestBuildWithoutPush() { "--tag", imageName, dir, } - err = common.RunBuild(args) + err = common.RunBuild(args, common.RunBuildStreams{}) require.Error(s.T(), err) require.Contains(s.T(), err.Error(), "please specify --push") } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 6ae48afa..ed323108 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -52,7 +52,7 @@ func addCommands(cmd *cobra.Command, streams genericclioptions.IOStreams) { //stopCmd(streams, opts), //installCmd(streams), //uninstallCmd(streams), - versionCmd(streams), + versionCmd(streams, opts), //pruneCmd(streams, opts), //duCmd(streams, opts), //imagetoolscmd.RootCmd(streams), diff --git a/pkg/cmd/version.go b/pkg/cmd/version.go index 2102065b..e001c24e 100644 --- a/pkg/cmd/version.go +++ b/pkg/cmd/version.go @@ -3,24 +3,68 @@ package commands import ( + "context" "fmt" + "github.com/moby/buildkit/util/appcontext" "github.com/spf13/cobra" + "github.com/vmware-tanzu/buildkit-cli-for-kubectl/pkg/driver" "github.com/vmware-tanzu/buildkit-cli-for-kubectl/version" "k8s.io/cli-runtime/pkg/genericclioptions" ) -func runVersion(streams genericclioptions.IOStreams) error { - fmt.Println(version.Version) +type versionOptions struct { + builder string + commonKubeOptions +} + +func getBuilderVersion(ctx context.Context, in versionOptions) string { + driverName := in.builder + if driverName == "" { + driverName = "buildkit" + } + d, err := driver.GetDriver(ctx, driverName, nil, in.KubeClientConfig, []string{}, "" /* unused config file */, map[string]string{} /* DriverOpts unused */, "") + if err != nil { + return err.Error() + } + version, err := d.GetVersion(ctx) + if err != nil { + return err.Error() + } + return version +} + +func runVersion(streams genericclioptions.IOStreams, in versionOptions) error { + ctx := appcontext.Context() + builderVersion := getBuilderVersion(ctx, in) + + fmt.Fprintf(streams.Out, "Client: %s\n", version.Version) + fmt.Fprintf(streams.Out, "Builder: %s\n", builderVersion) return nil } -func versionCmd(streams genericclioptions.IOStreams) *cobra.Command { +func versionCmd(streams genericclioptions.IOStreams, rootOpts *rootOptions) *cobra.Command { + options := versionOptions{ + commonKubeOptions: commonKubeOptions{ + configFlags: genericclioptions.NewConfigFlags(true), + IOStreams: streams, + }, + } + cmd := &cobra.Command{ Use: "version", - Short: "Show version information ", + Short: "Show client and builder version information ", RunE: func(cmd *cobra.Command, args []string) error { - return runVersion(streams) + options.builder = rootOpts.builder + + if err := options.Complete(cmd, args); err != nil { + return err + } + if err := options.Validate(); err != nil { + return err + } + + return runVersion(streams, options) }, } return cmd diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 3595ebf2..6341527c 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -81,6 +81,7 @@ type Driver interface { Features() map[Feature]bool List(ctx context.Context) ([]Builder, error) RuntimeSockProxy(ctx context.Context, name string) (net.Conn, error) + GetVersion(ctx context.Context) (string, error) // TODO - do we really need both? Seems like some cleanup needed here... GetAuthWrapper(string) imagetools.Auth diff --git a/pkg/driver/kubernetes/driver.go b/pkg/driver/kubernetes/driver.go index c5d00bbf..663ba7f6 100644 --- a/pkg/driver/kubernetes/driver.go +++ b/pkg/driver/kubernetes/driver.go @@ -3,10 +3,13 @@ package kubernetes import ( + "bytes" "context" "fmt" "net" + "os" "strconv" + "strings" "time" "github.com/moby/buildkit/client" @@ -22,9 +25,11 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" clientappsv1 "k8s.io/client-go/kubernetes/typed/apps/v1" clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/remotecommand" ) const ( @@ -262,6 +267,55 @@ func (d *Driver) RuntimeSockProxy(ctx context.Context, name string) (net.Conn, e } +func (d *Driver) GetVersion(ctx context.Context) (string, error) { + restClient := d.clientset.CoreV1().RESTClient() + restClientConfig, err := d.KubeClientConfig.ClientConfig() + if err != nil { + return "", err + } + pod, _, err := d.podChooser.ChoosePod(ctx) + if err != nil { + return "", err + } + if len(pod.Spec.Containers) == 0 { + return "", errors.Errorf("pod %s does not have any container", pod.Name) + } + containerName := pod.Spec.Containers[0].Name + cmd := []string{"buildkitd", "--version"} + buf := &bytes.Buffer{} + + req := restClient. + Post(). + Namespace(pod.Namespace). + Resource("pods"). + Name(pod.Name). + SubResource("exec"). + VersionedParams(&corev1.PodExecOptions{ + Container: containerName, + Command: cmd, + Stdin: false, + Stdout: true, + Stderr: true, + TTY: false, + }, scheme.ParameterCodec) + u := req.URL() + exec, err := remotecommand.NewSPDYExecutor(restClientConfig, "POST", u) + if err != nil { + return "", err + } + // TODO how to timeout if something goes bad...? + serr := exec.Stream(remotecommand.StreamOptions{ + Stdout: buf, + Stderr: os.Stderr, + Tty: false, + }) + if serr != nil { + return "", err + } + + return strings.TrimSpace(buf.String()), err +} + func (d *Driver) List(ctx context.Context) ([]driver.Builder, error) { var builders []driver.Builder depls, err := d.deploymentClient.List(ctx, metav1.ListOptions{}) diff --git a/pkg/driver/kubernetes/podchooser/podchooser.go b/pkg/driver/kubernetes/podchooser/podchooser.go index 5dd4b04e..b19b8038 100644 --- a/pkg/driver/kubernetes/podchooser/podchooser.go +++ b/pkg/driver/kubernetes/podchooser/podchooser.go @@ -57,6 +57,9 @@ func (pc *StickyPodChooser) ChoosePod(ctx context.Context) (*corev1.Pod, []*core if err != nil { return nil, nil, err } + if len(pods) == 0 { + return nil, nil, fmt.Errorf("no builder pods are running") + } var podNames []string podMap := make(map[string]*corev1.Pod, len(pods)) for _, pod := range pods { @@ -89,9 +92,13 @@ func (pc *StickyPodChooser) ChoosePod(ctx context.Context) (*corev1.Pod, []*core } func ListRunningPods(ctx context.Context, client clientcorev1.PodInterface, depl *appsv1.Deployment) ([]*corev1.Pod, error) { + name := depl.ObjectMeta.Name + if name == "" { + name = "buildkit" // TODO should be constant someplace... + } labelSelector := &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": depl.ObjectMeta.Name, + "app": name, }, } selector, err := metav1.LabelSelectorAsSelector(labelSelector)