Skip to content

Commit

Permalink
Refactor and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@tejal29
tejal29 committed May 10, 2019
1 parent d303b00 commit 5bfcc15
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 176 deletions.
25 changes: 18 additions & 7 deletions pkg/skaffold/deploy/helm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
Expand All @@ -32,6 +33,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
Expand Down Expand Up @@ -70,7 +72,7 @@ func (h *HelmDeployer) Labels() map[string]string {
}

func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) error {
var dRes []Artifact
var manifests kubectl.ManifestList

event.DeployInProgress()

Expand All @@ -83,13 +85,16 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build
return errors.Wrapf(err, "deploying %s", releaseName)
}

dRes = append(dRes, results...)
manifests.Append(results)
}

event.DeployComplete()

labels := merge(labellers...)
labelDeployResults(labels, dRes)
manifests, err := manifests.SetLabels(merge(labellers...))
if err != nil {
event.DeployFailed(err)
return errors.Wrap(err, "setting labels in manifests")
}

return nil
}
Expand Down Expand Up @@ -143,7 +148,7 @@ func (h *HelmDeployer) helm(ctx context.Context, out io.Writer, useSecrets bool,
return util.RunCmd(cmd)
}

func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r latest.HelmRelease, builds []build.Artifact) ([]Artifact, error) {
func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r latest.HelmRelease, builds []build.Artifact) (kubectl.ManifestBytes, error) {
isInstalled := true

releaseName, err := evaluateReleaseName(r.Name)
Expand Down Expand Up @@ -369,13 +374,19 @@ func (h *HelmDeployer) getReleaseInfo(ctx context.Context, release string) (*buf
// Retrieve info about all releases using helm get
// Skaffold labels will be applied to each deployed k8s object
// Since helm isn't always consistent with retrieving results, don't return errors here
func (h *HelmDeployer) getDeployResults(ctx context.Context, namespace string, release string) []Artifact {
func (h *HelmDeployer) getDeployResults(ctx context.Context, namespace string, release string) kubectl.ManifestBytes {
b, err := h.getReleaseInfo(ctx, release)
if err != nil {
logrus.Warnf(err.Error())
return nil
}
return parseReleaseInfo(namespace, b)
var manifest kubectl.ManifestBytes
manifest, err = ioutil.ReadAll(b)
if err != nil {
logrus.Warnf(err.Error())
return nil
}
return manifest
}

func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r latest.HelmRelease) error {
Expand Down
82 changes: 52 additions & 30 deletions pkg/skaffold/deploy/helm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ spec:
containers:
- name: skaffold-helm
image: gcr.io/nick-cloudbuild/skaffold-helm:f759510436c8fd6f7ffa13dd9e9d85e64bec8d2bfd12c5aa3fb9af1288eccdab
imagePullPolicy:
imagePullPolicy:
command: ["/bin/bash", "-c", "--" ]
args: ["while true; do sleep 30; done;"]
resources:
Expand Down Expand Up @@ -529,35 +529,6 @@ func (m *MockHelm) RunCmd(c *exec.Cmd) error {
}
}

func TestParseHelmRelease(t *testing.T) {
var tests = []struct {
name string
yaml []byte
shouldErr bool
}{
{
name: "parse valid deployment yaml",
yaml: []byte(validDeployYaml),
},
{
name: "parse valid service yaml",
yaml: []byte(validServiceYaml),
},
{
name: "parse invalid deployment yaml",
yaml: []byte(invalidDeployYaml),
shouldErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := parseRuntimeObject(testNamespace, tt.yaml)
testutil.CheckError(t, tt.shouldErr, err)
})
}
}

func TestExtractChartFilename(t *testing.T) {
out, err := extractChartFilename(
"Successfully packaged chart and saved it to: /var/folders/gm/rrs_712142x8vymmd7xq7h340000gn/T/foo-1.2.3-dirty.tgz\n",
Expand Down Expand Up @@ -634,3 +605,54 @@ func makeRunContext(helmDeploy *latest.HelmDeploy, force bool) *runcontext.RunCo
},
}
}

func TestHelmSetLabels(t *testing.T) {
var tests = []struct {
description string
files []string
valuesFiles []string
expected func(folder *testutil.TempDir) []string
}{
{
description: "charts dir is excluded",
files: []string{"Chart.yaml", "charts/xyz.tar", "templates/deploy.yaml"},
expected: func(folder *testutil.TempDir) []string {
return []string{folder.Path("Chart.yaml"), folder.Path("templates/deploy.yaml")}
},
},
{
description: "values file is included",
files: []string{"Chart.yaml"},
valuesFiles: []string{"/folder/values.yaml"},
expected: func(folder *testutil.TempDir) []string {
return []string{"/folder/values.yaml", folder.Path("Chart.yaml")}
},
},
}

for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
folder, cleanup := testutil.NewTempDir(t)
defer cleanup()
for _, file := range tt.files {
folder.Write(file, "")
}
deployer := NewHelmDeployer(makeRunContext(&latest.HelmDeploy{
Releases: []latest.HelmRelease{
{
Name: "skaffold-helm",
ChartPath: folder.Root(),
ValuesFiles: tt.valuesFiles,
Values: map[string]string{"image": "skaffold-helm"},
Overrides: schemautil.HelmOverrides{Values: map[string]interface{}{"foo": "bar"}},
SetValues: map[string]string{"some.key": "somevalue"},
},
},
}, false))

deps, err := deployer.Dependencies()

testutil.CheckErrorAndDeepEqual(t, false, err, tt.expected(folder), deps)
})
}
}
5 changes: 4 additions & 1 deletion pkg/skaffold/deploy/kubectl/manifests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ import (
"strings"
)

// ManifestBytes is a list of yaml bytes.
type ManifestBytes []byte

// ManifestList is a list of yaml manifests.
type ManifestList [][]byte
type ManifestList []ManifestBytes

func (l *ManifestList) String() string {
var str string
Expand Down
138 changes: 0 additions & 138 deletions pkg/skaffold/deploy/labels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,7 @@ limitations under the License.
package deploy

import (
"encoding/json"
"fmt"
"strings"
"time"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
patch "k8s.io/apimachinery/pkg/util/strategicpatch"
"k8s.io/client-go/discovery"
"k8s.io/client-go/dynamic"
)

// Artifact contains all information about a completed deployment
Expand All @@ -59,128 +43,6 @@ func merge(sources ...Labeller) map[string]string {
return merged
}

// retry 3 times to give the object time to propagate to the API server
const (
tries = 3
sleeptime = 300 * time.Millisecond
)

func labelDeployResults(labels map[string]string, results []Artifact) {
// use the kubectl client to update all k8s objects with a skaffold watermark
dynClient, err := kubernetes.DynamicClient()
if err != nil {
logrus.Warnf("error retrieving kubernetes dynamic client: %s", err.Error())
return
}

client, err := kubernetes.GetClientset()
if err != nil {
logrus.Warnf("error retrieving kubernetes client: %s", err.Error())
return
}

for _, res := range results {
err = nil
for i := 0; i < tries; i++ {
if err = updateRuntimeObject(dynClient, client.Discovery(), labels, res); err == nil {
break
}
time.Sleep(sleeptime)
}
if err != nil {
logrus.Warnf("error adding label to runtime object: %s", err.Error())
}
}
}

func addLabels(labels map[string]string, accessor metav1.Object) {
kv := make(map[string]string)

copyMap(kv, accessor.GetLabels())
copyMap(kv, labels)

accessor.SetLabels(kv)
}

func updateRuntimeObject(client dynamic.Interface, disco discovery.DiscoveryInterface, labels map[string]string, res Artifact) error {
originalJSON, _ := json.Marshal(res.Obj)
modifiedObj := res.Obj.DeepCopyObject()
accessor, err := meta.Accessor(modifiedObj)
if err != nil {
return errors.Wrap(err, "getting metadata accessor")
}
name := accessor.GetName()

kind := modifiedObj.GetObjectKind().GroupVersionKind().Kind
if strings.EqualFold(kind, "Service") {
logrus.Debugf("Labels are not applied to service [%s] because of issue: https://github.com/GoogleContainerTools/skaffold/issues/887", name)
return nil
}

addLabels(labels, accessor)

modifiedJSON, _ := json.Marshal(modifiedObj)
p, _ := patch.CreateTwoWayMergePatch(originalJSON, modifiedJSON, modifiedObj)
gvr, err := groupVersionResource(disco, modifiedObj.GetObjectKind().GroupVersionKind())
if err != nil {
return errors.Wrap(err, "getting group version resource from obj")
}

var namespace string
if accessor.GetNamespace() != "" {
namespace = accessor.GetNamespace()
} else {
namespace = res.Namespace
}

ns, err := resolveNamespace(namespace)
if err != nil {
return errors.Wrap(err, "resolving namespace")
}
logrus.Debugln("Patching", name, "in namespace", ns)

if _, err := client.Resource(gvr).Namespace(ns).Patch(name, types.StrategicMergePatchType, p); err != nil {
return errors.Wrapf(err, "patching resource %s/%s", namespace, name)
}

return nil
}

func resolveNamespace(ns string) (string, error) {
if ns != "" {
return ns, nil
}
cfg, err := kubectx.CurrentConfig()
if err != nil {
return "", errors.Wrap(err, "getting kubeconfig")
}

current, present := cfg.Contexts[cfg.CurrentContext]
if present && current.Namespace != "" {
return current.Namespace, nil
}
return "default", nil
}

func groupVersionResource(disco discovery.DiscoveryInterface, gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
resources, err := disco.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
if err != nil {
return schema.GroupVersionResource{}, errors.Wrap(err, "getting server resources for group version")
}

for _, r := range resources.APIResources {
if r.Kind == gvk.Kind {
return schema.GroupVersionResource{
Group: gvk.Group,
Version: gvk.Version,
Resource: r.Name,
}, nil
}
}

return schema.GroupVersionResource{}, fmt.Errorf("could not find resource for %s", gvk.String())
}

func copyMap(dest, from map[string]string) {
for k, v := range from {
dest[k] = v
Expand Down
Loading

0 comments on commit 5bfcc15

Please sign in to comment.