Merge pull request #3085 from chrislovecnm/file-asset-tasks

Automatic merge from submit-queue

starting work on file assets builder

I refactored to the dockerassets pkg to assetstasks, in order to not add yet another package.  Added file copy task, that I have tested with s3 locally, but not certain how to add memfs tests.

Fixes: #3086

Makefile

@@ -136,7 +136,7 @@ codegen: kops-gobindata
 	go install k8s.io/kops/upup/tools/generators/...
 	PATH=${GOPATH_1ST}/bin:${PATH} go generate k8s.io/kops/upup/pkg/fi/cloudup/awstasks
 	PATH=${GOPATH_1ST}/bin:${PATH} go generate k8s.io/kops/upup/pkg/fi/cloudup/gcetasks
-	PATH=${GOPATH_1ST}/bin:${PATH} go generate k8s.io/kops/upup/pkg/fi/dockertasks
+	PATH=${GOPATH_1ST}/bin:${PATH} go generate k8s.io/kops/upup/pkg/fi/assettasks
 	PATH=${GOPATH_1ST}/bin:${PATH} go generate k8s.io/kops/upup/pkg/fi/fitasks
 
 .PHONY: protobuf

cmd/kops/create_cluster.go

@@ -871,7 +871,7 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
 		return err
 	}
 
-	assetBuilder := assets.NewAssetBuilder()
+	assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
 	fullCluster, err := cloudup.PopulateClusterSpec(cluster, assetBuilder)
 	if err != nil {
 		return err

cmd/kops/edit_cluster.go

@@ -210,7 +210,7 @@ func RunEditCluster(f *util.Factory, cmd *cobra.Command, args []string, out io.W
 			return preservedFile(fmt.Errorf("error populating configuration: %v", err), file, out)
 		}
 
-		assetBuilder := assets.NewAssetBuilder()
+		assetBuilder := assets.NewAssetBuilder(newCluster.Spec.Assets)
 		fullCluster, err := cloudup.PopulateClusterSpec(newCluster, assetBuilder)
 		if err != nil {
 			results = editResults{

cmd/kops/edit_instancegroup.go

@@ -167,7 +167,7 @@ func RunEditInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string, ou
 		return fmt.Errorf("error populating configuration: %v", err)
 	}
 
-	assetBuilder := assets.NewAssetBuilder()
+	assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
 	fullCluster, err := cloudup.PopulateClusterSpec(cluster, assetBuilder)
 	if err != nil {
 		return err

cmd/kops/upgrade_cluster.go

@@ -289,7 +289,7 @@ func (c *UpgradeClusterCmd) Run(args []string) error {
 			return fmt.Errorf("error populating configuration: %v", err)
 		}
 
-		assetBuilder := assets.NewAssetBuilder()
+		assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
 		fullCluster, err := cloudup.PopulateClusterSpec(cluster, assetBuilder)
 		if err != nil {
 			return err

hack/.packages

@@ -95,6 +95,7 @@ k8s.io/kops/tests/integration/channel
 k8s.io/kops/tests/integration/conversion
 k8s.io/kops/upup/models
 k8s.io/kops/upup/pkg/fi
+k8s.io/kops/upup/pkg/fi/assettasks
 k8s.io/kops/upup/pkg/fi/cloudup
 k8s.io/kops/upup/pkg/fi/cloudup/awstasks
 k8s.io/kops/upup/pkg/fi/cloudup/awsup
@@ -106,7 +107,6 @@ k8s.io/kops/upup/pkg/fi/cloudup/gcetasks
 k8s.io/kops/upup/pkg/fi/cloudup/terraform
 k8s.io/kops/upup/pkg/fi/cloudup/vsphere
 k8s.io/kops/upup/pkg/fi/cloudup/vspheretasks
-k8s.io/kops/upup/pkg/fi/dockertasks
 k8s.io/kops/upup/pkg/fi/fitasks
 k8s.io/kops/upup/pkg/fi/k8sapi
 k8s.io/kops/upup/pkg/fi/loader

pkg/assets/builder.go

@@ -19,9 +19,11 @@ package assets
 import (
 	"bytes"
 	"fmt"
+	"net/url"
 	"os"
 	"strings"
 
+	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/pkg/kubemanifest"
 )
@@ -32,19 +34,31 @@ var RewriteManifests = featureflag.New("RewriteManifests", featureflag.Bool(true
 
 // AssetBuilder discovers and remaps assets
 type AssetBuilder struct {
-	Assets []*Asset
+	ContainerAssets []*ContainerAsset
+	FileAssets      []*FileAsset
+	AssetsLocation  *kops.Assets
 }
 
-type Asset struct {
+type ContainerAsset struct {
 	// DockerImage will be the name of the docker image we should run, if this is a docker image
 	DockerImage string
 
 	// CanonicalLocation will be the source location of the image, if we should copy it to the actual location
 	CanonicalLocation string
 }
 
-func NewAssetBuilder() *AssetBuilder {
-	return &AssetBuilder{}
+type FileAsset struct {
+	// File will be the name of the file we should use
+	File string
+
+	// CanonicalLocation will be the source location of the file, if we should copy it to the actual location
+	CanonicalLocation string
+}
+
+func NewAssetBuilder(assets *kops.Assets) *AssetBuilder {
+	return &AssetBuilder{
+		AssetsLocation: assets,
+	}
 }
 
 // RemapManifest transforms a kubernetes manifest.
@@ -79,7 +93,7 @@ func (a *AssetBuilder) RemapManifest(data []byte) ([]byte, error) {
 }
 
 func (a *AssetBuilder) RemapImage(image string) (string, error) {
-	asset := &Asset{}
+	asset := &ContainerAsset{}
 
 	asset.DockerImage = image
 
@@ -113,7 +127,33 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) {
 		image = asset.DockerImage
 	}
 
-	a.Assets = append(a.Assets, asset)
+	a.ContainerAssets = append(a.ContainerAssets, asset)
 
 	return image, nil
 }
+
+// RemapFile sets a new url location for the file, if a AssetsLocation is defined.
+func (a AssetBuilder) RemapFile(file string) (string, error) {
+	if file == "" {
+		return "", fmt.Errorf("unable to remap an empty string")
+	}
+
+	fileAsset := &FileAsset{
+		File:              file,
+		CanonicalLocation: file,
+	}
+
+	if a.AssetsLocation != nil && a.AssetsLocation.FileRepository != nil {
+		fileURL, err := url.Parse(file)
+		if err != nil {
+			return "", fmt.Errorf("unable to parse file url %q: %v", file, err)
+		}
+
+		fileRepo := strings.TrimSuffix(*a.AssetsLocation.FileRepository, "/")
+		fileAsset.File = fileRepo + fileURL.Path
+	}
+
+	a.FileAssets = append(a.FileAssets, fileAsset)
+
+	return fileAsset.File, nil
+}

pkg/assets/builder_test.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2017 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 assets
+
+import (
+	"k8s.io/kops/pkg/apis/kops"
+	"testing"
+)
+
+func TestRemap_File(t *testing.T) {
+	grid := []struct {
+		testFile   string
+		expected   string
+		asset      *FileAsset
+		kopsAssets *kops.Assets
+	}{
+		{
+			// FIXME - need https://s3.amazonaws.com/k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet
+			"https://gcr.io/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
+			"s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
+			&FileAsset{
+				File:              "s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
+				CanonicalLocation: "https://gcr.io/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
+			},
+			&kops.Assets{
+				FileRepository: s("s3://k8s-for-greeks-kops"),
+			},
+		},
+	}
+
+	for _, g := range grid {
+		builder := NewAssetBuilder(g.kopsAssets)
+
+		actual, err := builder.RemapFile(g.testFile)
+		if err != nil {
+			t.Errorf("err occurred: %v", err)
+		}
+		if actual != g.expected {
+			t.Errorf("results did not match.  actual=%q expected=%q", actual, g.expected)
+		}
+	}
+}
+
+func s(s string) *string {
+	return &s
+}

upup/pkg/fi/dockertasks/copydockerimage.go → upup/pkg/fi/assettasks/copydockerimage.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package dockertasks
+package assettasks
 
 import (
 	"fmt"
@@ -30,6 +30,7 @@ type CopyDockerImage struct {
 	Name        *string
 	SourceImage *string
 	TargetImage *string
+	Lifecycle   *fi.Lifecycle
 }
 
 var _ fi.CompareWithID = &CopyDockerImage{}