Skip to content

Commit

Permalink
refactoring to using using builder
Browse files Browse the repository at this point in the history
  • Loading branch information
chrislovecnm committed Aug 1, 2017
1 parent 57ced39 commit 6a77fa7
Show file tree
Hide file tree
Showing 17 changed files with 84 additions and 117 deletions.
2 changes: 1 addition & 1 deletion cmd/kops/create_cluster.go
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_cluster.go
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_instancegroup.go
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/upgrade_cluster.go
Expand Up @@ -284,7 +284,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
Expand Down
52 changes: 46 additions & 6 deletions pkg/assets/builder.go
Expand Up @@ -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"
)
Expand All @@ -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.
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Up @@ -18,7 +18,6 @@ package assets

import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/upup/pkg/fi"
"testing"
)

Expand All @@ -38,13 +37,13 @@ func TestRemap_File(t *testing.T) {
CanonicalLocation: "https://gcr.io/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
},
&kops.Assets{
FileRepository: fi.String("s3://k8s-for-greeks-kops"),
FileRepository: s("s3://k8s-for-greeks-kops"),
},
},
}

for _, g := range grid {
builder := NewFileAssetBuilder(g.kopsAssets)
builder := NewAssetBuilder(g.kopsAssets)

actual, err := builder.RemapFile(g.testFile)
if err != nil {
Expand All @@ -55,3 +54,7 @@ func TestRemap_File(t *testing.T) {
}
}
}

func s(s string) *string {
return &s
}
76 changes: 0 additions & 76 deletions pkg/assets/file_builder.go

This file was deleted.

8 changes: 4 additions & 4 deletions upup/pkg/fi/assettasks/copyfile.go
Expand Up @@ -116,10 +116,10 @@ func transferFile(source string, target string, sourceSHA string, sourceSHALocat

if sha != "" {

trimmedSHA := strings.TrimSuffix(sha, "\n")
trimmedSHA := strings.TrimSpace(sha)

in := bytes.NewReader(data)
dataHash, err := hashing.HashesForResource(in, []hashing.HashAlgorithm{hashing.HashAlgorithmSHA1})
dataHash, err := hashing.HashAlgorithmSHA1.Hash(in)
if err != nil {
return fmt.Errorf("unable to hash sha from data: %v", err)
}
Expand All @@ -129,8 +129,8 @@ func transferFile(source string, target string, sourceSHA string, sourceSHALocat
return fmt.Errorf("unable to hash sha: %q, %v", sha, err)
}

if !shaHash.Equal(dataHash[0]) {
return fmt.Errorf("SHAs are not matching for %q", dataHash[0].String())
if !shaHash.Equal(dataHash) {
return fmt.Errorf("SHAs are not matching for %q", dataHash.String())
}

shaVFS, err := buildVFSPath(target + ".sha1")
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/apply_cluster.go
Expand Up @@ -143,7 +143,7 @@ func (c *ApplyClusterCmd) Run() error {
}
c.channel = channel

assetBuilder := assets.NewAssetBuilder()
assetBuilder := assets.NewAssetBuilder(c.Cluster.Spec.Assets)
err = c.upgradeSpecs(assetBuilder)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/awstasks/elastic_ip_test.go
Expand Up @@ -104,7 +104,7 @@ func TestElasticIPCreate(t *testing.T) {
}

func checkNoChanges(t *testing.T, cloud fi.Cloud, allTasks map[string]fi.Task) {
assetBuilder := assets.NewAssetBuilder()
assetBuilder := assets.NewAssetBuilder(nil)
target := fi.NewDryRunTarget(assetBuilder, os.Stderr)
context, err := fi.NewContext(target, cloud, nil, nil, nil, true, allTasks)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go
Expand Up @@ -64,7 +64,7 @@ func runChannelBuilderTest(t *testing.T, key string) {
t.Fatalf("error from PerformAssignments: %v", err)
}

assetBuilder := assets.NewAssetBuilder()
assetBuilder := assets.NewAssetBuilder(nil)
fullSpec, err := PopulateClusterSpec(cluster, assetBuilder)
if err != nil {
t.Fatalf("error from PopulateClusterSpec: %v", err)
Expand All @@ -81,7 +81,7 @@ func runChannelBuilderTest(t *testing.T, key string) {
bcb := BootstrapChannelBuilder{
cluster: cluster,
templates: templates,
assetBuilder: assets.NewAssetBuilder(),
assetBuilder: assets.NewAssetBuilder(nil),
}

context := &fi.ModelBuilderContext{
Expand Down
4 changes: 2 additions & 2 deletions upup/pkg/fi/cloudup/loader.go
Expand Up @@ -181,7 +181,7 @@ func (l *Loader) BuildTasks(modelStore vfs.Path, models []string, assetBuilder *
l.tasks = context.Tasks
}

if err := l.addAssetCopyTasks(assetBuilder.Assets); err != nil {
if err := l.addAssetCopyTasks(assetBuilder.ContainerAssets); err != nil {
return nil, err
}

Expand All @@ -192,7 +192,7 @@ func (l *Loader) BuildTasks(modelStore vfs.Path, models []string, assetBuilder *
return l.tasks, nil
}

func (l *Loader) addAssetCopyTasks(assets []*assets.Asset) error {
func (l *Loader) addAssetCopyTasks(assets []*assets.ContainerAsset) error {
for _, asset := range assets {
if asset.CanonicalLocation != "" && asset.DockerImage != asset.CanonicalLocation {
context := &fi.ModelBuilderContext{
Expand Down

0 comments on commit 6a77fa7

Please sign in to comment.