cleanup, improve cover

cyrildiagne · Jan 3, 2020 · b27a9ff · b27a9ff
1 parent 815cc92
commit b27a9ff
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 54 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+}