Skip to content

Commit

Permalink
refactoring to deal with hash
Browse files Browse the repository at this point in the history
  • Loading branch information
chrislovecnm committed Oct 13, 2017
1 parent d43566d commit 8624015
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 137 deletions.
2 changes: 1 addition & 1 deletion cmd/kops/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -943,7 +943,7 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
return err
}

assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets, "")
fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,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(newCluster.Spec.Assets)
assetBuilder := assets.NewAssetBuilder(newCluster.Spec.Assets, "")
fullCluster, err := cloudup.PopulateClusterSpec(clientset, newCluster, assetBuilder)
if err != nil {
results = editResults{
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func RunEditInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string, ou
return fmt.Errorf("error populating configuration: %v", err)
}

assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets, "")
fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/upgrade_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func (c *UpgradeClusterCmd) Run(args []string) error {
return fmt.Errorf("error populating configuration: %v", err)
}

assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets)
assetBuilder := assets.NewAssetBuilder(cluster.Spec.Assets, "")
fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder)
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions pkg/assets/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ go_library(
"//pkg/apis/kops:go_default_library",
"//pkg/featureflag:go_default_library",
"//pkg/kubemanifest:go_default_library",
"//util/pkg/hashing:go_default_library",
"//util/pkg/vfs:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
],
)

Expand Down
60 changes: 49 additions & 11 deletions pkg/assets/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/kubemanifest"
"k8s.io/kops/util/pkg/hashing"
"k8s.io/kops/util/pkg/vfs"
)

// RewriteManifests controls whether we rewrite manifests
Expand All @@ -38,6 +40,9 @@ type AssetBuilder struct {
ContainerAssets []*ContainerAsset
FileAssets []*FileAsset
AssetsLocation *kops.Assets
// yay go cyclic dependency
//Lifecycle *fi.Lifecycle
Lifecycle string
}

type ContainerAsset struct {
Expand Down Expand Up @@ -65,9 +70,10 @@ type FileAsset struct {
SHAValue string
}

func NewAssetBuilder(assets *kops.Assets) *AssetBuilder {
func NewAssetBuilder(assets *kops.Assets, lifecycle string) *AssetBuilder {
return &AssetBuilder{
AssetsLocation: assets,
Lifecycle: lifecycle,
}
}

Expand Down Expand Up @@ -141,9 +147,9 @@ func (a *AssetBuilder) RemapImage(image string) (string, error) {
}

// RemapFile sets a new url location for the file, if a AssetsLocation is defined.
func (a *AssetBuilder) RemapFileAndSHA(file string, sha string) (string, error) {
func (a *AssetBuilder) RemapFileAndSHA(file string, sha string) (string, *hashing.Hash, error) {
if file == "" {
return "", fmt.Errorf("unable to remap an empty string")
return "", nil, fmt.Errorf("unable to remap an empty string")
}

fileAsset := &FileAsset{
Expand All @@ -157,27 +163,31 @@ func (a *AssetBuilder) RemapFileAndSHA(file string, sha string) (string, error)

file, err := a.normalizeURL(file)
if err != nil {
return "", fmt.Errorf("unable to parse file url %q: %v", file, err)
return "", nil, fmt.Errorf("unable to parse file url %q: %v", file, err)
}

fileAsset.File = file

sha, err = a.normalizeURL(sha)
if err != nil {
return "", fmt.Errorf("unable to parse sha url %q: %v", file, err)
return "", nil, fmt.Errorf("unable to parse sha url %q: %v", file, err)
}

fileAsset.SHA = sha
glog.V(4).Infof("adding remapped file: %q", fileAsset.File)
}

a.FileAssets = append(a.FileAssets, fileAsset)
return fileAsset.File, nil
h, err := a.FindHash(fileAsset)
if err != nil {
return "", nil, err
}
return fileAsset.File, h, nil
}

func (a *AssetBuilder) RemapFileAndSHAValue(file string, shaValue string) (string, error) {
func (a *AssetBuilder) RemapFileAndSHAValue(file string, shaValue string) (string, *hashing.Hash, error) {
if file == "" {
return "", fmt.Errorf("unable to remap an empty string")
return "", nil, fmt.Errorf("unable to remap an empty string")
}

fileAsset := &FileAsset{
Expand All @@ -190,15 +200,44 @@ func (a *AssetBuilder) RemapFileAndSHAValue(file string, shaValue string) (strin

file, err := a.normalizeURL(file)
if err != nil {
return "", fmt.Errorf("unable to parse file url %q: %v", file, err)
return "", nil, fmt.Errorf("unable to parse file url %q: %v", file, err)
}

fileAsset.File = file
glog.V(4).Infof("adding remapped file: %q", fileAsset.File)
}

a.FileAssets = append(a.FileAssets, fileAsset)
return fileAsset.File, nil
h, err := a.FindHash(fileAsset)
if err != nil {
return "", nil, err
}
return fileAsset.File, h, nil
}

// TODO we should probably make this a task....

func (a *AssetBuilder) FindHash(file *FileAsset) (*hashing.Hash, error) {
u := file.File

// FIXME ugly hack because dep loop with lifecycle
if a.Lifecycle == string("Sync") {
u = file.CanonicalLocation
}

for _, ext := range []string{".sha1"} {
hashURL := u + ext
b, err := vfs.Context.ReadFile(hashURL)
if err != nil {
glog.Infof("error reading hash file %q: %v", hashURL, err)
continue
}
hashString := strings.TrimSpace(string(b))
glog.V(2).Infof("Found hash %q for %q", hashString, u)

return hashing.FromString(hashString)
}
return nil, fmt.Errorf("cannot determine hash for %q (have you specified a valid file location?)", u)
}

func (a *AssetBuilder) normalizeURL(file string) (string, error) {
Expand All @@ -209,5 +248,4 @@ func (a *AssetBuilder) normalizeURL(file string) (string, error) {

fileRepo := strings.TrimSuffix(*a.AssetsLocation.FileRepository, "/")
return fileRepo + fileURL.Path, nil

}
19 changes: 13 additions & 6 deletions pkg/assets/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,42 @@ limitations under the License.
package assets

import (
"k8s.io/kops/pkg/apis/kops"
"testing"

"k8s.io/kops/pkg/apis/kops"
)

func TestRemap_File(t *testing.T) {
t.Skip("not going to run")
grid := []struct {
testFile string
expected string
sha string
asset *FileAsset
kopsAssets *kops.Assets
}{
//defaultCNIAssetK8s1_6 = "https://storage.googleapis.com/kubernetes-release/network-plugins/cni-0799f5732f2a11b329d9e3d51b9c8f2e3759f2ff.tar.gz"
//defaultCNIAssetHashStringK8s1_6 = "1d9788b0f5420e1a219aad2cb8681823fc515e7c"
{
// 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",
"s3://clove-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
"1d9788b0f5420e1a219aad2cb8681823fc515e7c",
&FileAsset{
File: "s3://k8s-for-greeks-kops/kubernetes-release/release/v1.7.2/bin/linux/amd64/kubelet",
File: "s3://clove-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"),
FileRepository: s("s3://clove-kops"),
},
},
}

for _, g := range grid {
builder := NewAssetBuilder(g.kopsAssets)
// TODO FIXME
builder := NewAssetBuilder(g.kopsAssets, "")

actual, err := builder.RemapFile(g.testFile)
actual, _, err := builder.RemapFileAndSHAValue(g.testFile, g.sha)
if err != nil {
t.Errorf("err occurred: %v", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/components/kubecontrollermanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func buildCluster() *api.Cluster {

func Test_Build_KCM_Builder_Lower_Version(t *testing.T) {
versions := []string{"v1.4.0", "v1.4.7", "v1.5.0"}
b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil,"")

for _, v := range versions {

Expand Down Expand Up @@ -72,7 +72,7 @@ func Test_Build_KCM_Builder_Lower_Version(t *testing.T) {

func Test_Build_KCM_Builder_High_Enough_Version(t *testing.T) {
versions := []string{"v1.4.8", "v1.5.2", "v1.9.0", "v2.4.0"}
b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil, "")
for _, v := range versions {

c := buildCluster()
Expand Down Expand Up @@ -103,7 +103,7 @@ func Test_Build_KCM_Builder_Change_Duration(t *testing.T) {
c := buildCluster()
c.Spec.KubernetesVersion = "v1.5.2"

b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil,"")
kcm := &KubeControllerManagerOptionsBuilder{
Context: &OptionsContext{
AssetBuilder: b,
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/components/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func buildSpec() *kops.ClusterSpec {
}

func buildOptions(spec *kops.ClusterSpec) error {
ab := assets.NewAssetBuilder(nil)
ab := assets.NewAssetBuilder(nil,"")

ver, err := KubernetesVersion(spec)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/components/kubescheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func buildSchedulerConfigMapCluster() *api.Cluster {

func Test_Build_Scheduler_Without_PolicyConfigMap(t *testing.T) {
versions := []string{"v1.6.0", "v1.6.4", "v1.7.0", "v1.7.4"}
b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil,"")

for _, v := range versions {

Expand Down Expand Up @@ -70,7 +70,7 @@ func Test_Build_Scheduler_Without_PolicyConfigMap(t *testing.T) {
}
func Test_Build_Scheduler_PolicyConfigMap_Unsupported_Version(t *testing.T) {
versions := []string{"v1.6.0", "v1.6.4"}
b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil,"")

for _, v := range versions {

Expand Down Expand Up @@ -103,7 +103,7 @@ func Test_Build_Scheduler_PolicyConfigMap_Unsupported_Version(t *testing.T) {

func Test_Build_Scheduler_PolicyConfigMap_Supported_Version(t *testing.T) {
versions := []string{"v1.7.0", "v1.7.4", "v1.8.0"}
b := assets.NewAssetBuilder(nil)
b := assets.NewAssetBuilder(nil,"")

for _, v := range versions {

Expand Down
Loading

0 comments on commit 8624015

Please sign in to comment.