From b27a9fff25072aedc952c7a68ed10674313324be Mon Sep 17 00:00:00 2001 From: Cyril Diagne Date: Fri, 3 Jan 2020 12:30:53 +0100 Subject: [PATCH] cleanup, improve cover --- cmd/deploy.go | 41 ++++++++++--------- cmd/dev.go | 41 ++++++++++--------- pkg/kuda/deployer/skaffold/config/knative.go | 7 ++-- .../deployer/skaffold/config/knative_test.go | 20 ++++++++- pkg/kuda/deployer/skaffold/config/skaffold.go | 11 ----- .../deployer/skaffold/config/skaffold_test.go | 4 ++ .../deployer/skaffold/config/utils_test.go | 25 +++++++++++ 7 files changed, 95 insertions(+), 54 deletions(-) create mode 100644 pkg/kuda/deployer/skaffold/config/utils_test.go diff --git a/cmd/deploy.go b/cmd/deploy.go index 60f6a61..cf498ad 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -3,12 +3,12 @@ package cmd import ( "fmt" "os" - "os/exec" "path/filepath" skaffoldCfg "github.com/cyrildiagne/kuda/pkg/kuda/deployer/skaffold/config" "github.com/cyrildiagne/kuda/pkg/kuda/manifest/latest" "github.com/spf13/cobra" + yaml "gopkg.in/yaml.v2" ) // deployCmd represents the `kuda deploy` command. @@ -51,40 +51,43 @@ func deployWithSkaffold(manifestFile string) error { } // Generate the knative yaml file. - knative, err := skaffoldCfg.GenerateKnativeConfigYAML(manifest.Name, manifest.Deploy, cfg) + knativeCfg, err := skaffoldCfg.GenerateKnativeConfig(manifest.Name, manifest.Deploy, cfg) if err != nil { return err } - knativeFile := filepath.FromSlash(cfgFolder + "/knative.yaml") - err = writeYAML(knative, knativeFile) + knativeYAML, err := skaffoldCfg.MarshalKnativeConfig(knativeCfg) if err != nil { return err } + knativeFile := filepath.FromSlash(cfgFolder + "/knative.yaml") + if err := writeYAML(knativeYAML, knativeFile); err != nil { + return err + } // Generate the skaffold yaml file. - skaffold, err := skaffoldCfg.GenerateSkaffoldConfigYAML(manifest.Name, manifest.Deploy, cfg, knativeFile) + skaffoldCfg, err := skaffoldCfg.GenerateSkaffoldConfig(manifest.Name, manifest.Deploy, cfg, knativeFile) if err != nil { return err } - skaffoldFile := filepath.FromSlash(cfgFolder + "/skaffold.yaml") - if err := writeYAML(skaffold, skaffoldFile); err != nil { + skaffoldYAML, err := yaml.Marshal(skaffoldCfg) + if err != nil { return err } - - // Run command. - args := []string{"run", "-f", skaffoldFile} - cmd := exec.Command("skaffold", args...) - cmd.Stdout = os.Stdout - cmd.Stdin = os.Stdin - cmd.Stderr = os.Stderr - - if err := cmd.Run(); err != nil { + skaffoldFile := filepath.FromSlash(cfgFolder + "/skaffold.yaml") + if err := writeYAML(skaffoldYAML, skaffoldFile); err != nil { return err } - os.Stdout.Close() - os.Stdin.Close() - os.Stderr.Close() + // // Run command. + // args := []string{"run", "-f", skaffoldFile} + // cmd := exec.Command("skaffold", args...) + // cmd.Stdout = os.Stdout + // cmd.Stdin = os.Stdin + // cmd.Stderr = os.Stderr + + // if err := cmd.Run(); err != nil { + // return err + // } return nil } diff --git a/cmd/dev.go b/cmd/dev.go index 863d442..f624f26 100644 --- a/cmd/dev.go +++ b/cmd/dev.go @@ -3,12 +3,12 @@ package cmd import ( "fmt" "os" - "os/exec" "path/filepath" skaffoldCfg "github.com/cyrildiagne/kuda/pkg/kuda/deployer/skaffold/config" "github.com/cyrildiagne/kuda/pkg/kuda/manifest/latest" "github.com/spf13/cobra" + yaml "gopkg.in/yaml.v2" ) // devCmd represents the `kuda dev` command. @@ -51,40 +51,43 @@ func devWithSkaffold(manifestFile string) error { } // Generate the knative yaml file. - knative, err := skaffoldCfg.GenerateKnativeConfigYAML(manifest.Name+"-dev", manifest.Dev, cfg) + knativeCfg, err := skaffoldCfg.GenerateKnativeConfig(manifest.Name+"-dev", manifest.Dev, cfg) if err != nil { return err } - knativeFile := filepath.FromSlash(cfgFolder + "/knative-dev.yaml") - err = writeYAML(knative, knativeFile) + knativeYAML, err := skaffoldCfg.MarshalKnativeConfig(knativeCfg) if err != nil { return err } + knativeFile := filepath.FromSlash(cfgFolder + "/knative-dev.yaml") + if err := writeYAML(knativeYAML, knativeFile); err != nil { + return err + } // Generate the skaffold yaml file. - skaffold, err := skaffoldCfg.GenerateSkaffoldConfigYAML(manifest.Name+"-dev", manifest.Dev, cfg, knativeFile) + skaffoldCfg, err := skaffoldCfg.GenerateSkaffoldConfig(manifest.Name+"-dev", manifest.Dev, cfg, knativeFile) if err != nil { return err } - skaffoldFile := filepath.FromSlash(cfgFolder + "/skaffold-dev.yaml") - if err := writeYAML(skaffold, skaffoldFile); err != nil { + skaffoldYAML, err := yaml.Marshal(skaffoldCfg) + if err != nil { return err } - - // Run command. - args := []string{"dev", "-f", skaffoldFile} - cmd := exec.Command("skaffold", args...) - cmd.Stdout = os.Stdout - cmd.Stdin = os.Stdin - cmd.Stderr = os.Stderr - - if err := cmd.Run(); err != nil { + skaffoldFile := filepath.FromSlash(cfgFolder + "/skaffold-dev.yaml") + if err := writeYAML(skaffoldYAML, skaffoldFile); err != nil { return err } - os.Stdout.Close() - os.Stdin.Close() - os.Stderr.Close() + // // Run command. + // args := []string{"dev", "-f", skaffoldFile} + // cmd := exec.Command("skaffold", args...) + // cmd.Stdout = os.Stdout + // cmd.Stdin = os.Stdin + // cmd.Stderr = os.Stderr + + // if err := cmd.Run(); err != nil { + // return err + // } return nil } diff --git a/pkg/kuda/deployer/skaffold/config/knative.go b/pkg/kuda/deployer/skaffold/config/knative.go index 59913ef..686d9d0 100644 --- a/pkg/kuda/deployer/skaffold/config/knative.go +++ b/pkg/kuda/deployer/skaffold/config/knative.go @@ -12,10 +12,9 @@ import ( yaml "sigs.k8s.io/yaml" ) -// GenerateKnativeConfigYAML generate yaml string. -func GenerateKnativeConfigYAML(name string, manifest latest.Config, cfg config.UserConfig) ([]byte, error) { - config, _ := GenerateKnativeConfig(name, manifest, cfg) - content, err := yaml.Marshal(config) +// MarshalKnativeConfig generate yaml bytes from a knative config. +func MarshalKnativeConfig(s v1.Service) ([]byte, error) { + content, err := yaml.Marshal(s) if err != nil { return nil, err } diff --git a/pkg/kuda/deployer/skaffold/config/knative_test.go b/pkg/kuda/deployer/skaffold/config/knative_test.go index 588dfcd..68d86bd 100644 --- a/pkg/kuda/deployer/skaffold/config/knative_test.go +++ b/pkg/kuda/deployer/skaffold/config/knative_test.go @@ -18,6 +18,14 @@ func TestGenerateKnativeConfig(t *testing.T) { name := "test-name" cfg := latest.Config{ Dockerfile: "test-file", + Entrypoint: latest.Entrypoint{ + Command: "test-cmd", + Args: []string{"test-arg1", "test-arg2"}, + }, + Env: []corev1.EnvVar{{ + Name: "TEST_ENV_NAME", + Value: "test-env-value", + }}, } userCfg := config.UserConfig{ Namespace: "test-namespace", @@ -44,7 +52,9 @@ func TestGenerateKnativeConfig(t *testing.T) { t.Errorf("Mismatch (-want +got):\n%s", diff) } - numGPUs, _ := resource.ParseQuantity("1") + numGPUs, err := resource.ParseQuantity("1") + assert.NilError(t, err) + container := corev1.Container{ Image: "test-registry/test-name", Name: "test-name", @@ -53,6 +63,9 @@ func TestGenerateKnativeConfig(t *testing.T) { corev1.ResourceName("nvidia.com/gpu"): numGPUs, }, }, + Command: []string{"test-cmd"}, + Args: []string{"test-arg1", "test-arg2"}, + Env: []corev1.EnvVar{{Name: "TEST_ENV_NAME", Value: "test-env-value"}}, } spec := v1.ServiceSpec{ ConfigurationSpec: v1.ConfigurationSpec{ @@ -68,4 +81,9 @@ func TestGenerateKnativeConfig(t *testing.T) { if diff := cmp.Diff(result.Spec, spec); diff != "" { t.Errorf("Mismatch (-want +got):\n%s", diff) } + + // Test Marshal + resultYAML, err := MarshalKnativeConfig(result) + assert.NilError(t, err) + assert.Assert(t, len(resultYAML) > 0) } diff --git a/pkg/kuda/deployer/skaffold/config/skaffold.go b/pkg/kuda/deployer/skaffold/config/skaffold.go index acc840c..80a7fb1 100644 --- a/pkg/kuda/deployer/skaffold/config/skaffold.go +++ b/pkg/kuda/deployer/skaffold/config/skaffold.go @@ -4,19 +4,8 @@ import ( v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1" config "github.com/cyrildiagne/kuda/pkg/kuda/config" latest "github.com/cyrildiagne/kuda/pkg/kuda/manifest/latest" - yaml "gopkg.in/yaml.v2" ) -// GenerateSkaffoldConfigYAML generate yaml string. -func GenerateSkaffoldConfigYAML(name string, manifest latest.Config, cfg config.UserConfig, knativeFile string) ([]byte, error) { - config, _ := GenerateSkaffoldConfig(name, manifest, cfg, knativeFile) - content, err := yaml.Marshal(config) - if err != nil { - return nil, err - } - return content, nil -} - // GenerateSkaffoldConfig generate skaffold yaml specifics to the Kuda workflow. func GenerateSkaffoldConfig(name string, manifest latest.Config, userCfg config.UserConfig, knativeFile string) (v1.SkaffoldConfig, error) { diff --git a/pkg/kuda/deployer/skaffold/config/skaffold_test.go b/pkg/kuda/deployer/skaffold/config/skaffold_test.go index 41ef595..30e1102 100644 --- a/pkg/kuda/deployer/skaffold/config/skaffold_test.go +++ b/pkg/kuda/deployer/skaffold/config/skaffold_test.go @@ -15,6 +15,7 @@ func TestGenerateSkaffoldConfig(t *testing.T) { name := "test-name" cfg := latest.Config{ Dockerfile: "test-file", + Sync: []string{"test-sync"}, } userCfg := config.UserConfig{ Namespace: "test-namespace", @@ -42,6 +43,9 @@ func TestGenerateSkaffoldConfig(t *testing.T) { DockerfilePath: "test-file", }, }, + Sync: &v1.Sync{ + Manual: []*v1.SyncRule{{Src: "test-sync", Dest: "."}}, + }, }, } if diff := cmp.Diff(result.Pipeline.Build.Artifacts, artifacts); diff != "" { diff --git a/pkg/kuda/deployer/skaffold/config/utils_test.go b/pkg/kuda/deployer/skaffold/config/utils_test.go new file mode 100644 index 0000000..b470f36 --- /dev/null +++ b/pkg/kuda/deployer/skaffold/config/utils_test.go @@ -0,0 +1,25 @@ +package config + +import ( + "testing" + + config "github.com/cyrildiagne/kuda/pkg/kuda/config" + "gotest.tools/assert" +) + +func TestGetDockerfileArtifactName(t *testing.T) { + testUserCfg := config.UserConfig{ + Deployer: config.DeployerType{ + Skaffold: &config.SkaffoldDeployerConfig{ + DockerRegistry: "test-registry", + }, + }, + } + // Test without extension. + name := GetDockerfileArtifactName(testUserCfg, "test") + assert.Equal(t, "test-registry/test", name) + + // Test with "-dev" extension which should be removed in docker image name. + name = GetDockerfileArtifactName(testUserCfg, "test-dev") + assert.Equal(t, "test-registry/test", name) +}