Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cluster spec to node user data so component config changes are detected #3120

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions cmd/kops/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,20 @@ import (
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"golang.org/x/crypto/ssh"
"io/ioutil"
"k8s.io/kops/cmd/kops/util"
"k8s.io/kops/pkg/diff"
"k8s.io/kops/pkg/testutils"
"os"
"path"
"reflect"
"sort"
"strings"
"testing"
"time"

"golang.org/x/crypto/ssh"

"k8s.io/kops/cmd/kops/util"
"k8s.io/kops/pkg/diff"
"k8s.io/kops/pkg/testutils"
)

// TestMinimal runs the test on a minimum configuration, similar to kops create cluster minimal.example.com --zones us-west-1a
Expand Down Expand Up @@ -322,8 +324,10 @@ func runTestCloudformation(t *testing.T, clusterName string, srcDir string, vers
t.Fatalf("unexpected error reading expected cloudformation output: %v", err)
}

if !bytes.Equal(actualCF, expectedCF) {
diffString := diff.FormatDiff(string(expectedCF), string(actualCF))
expectedCFTrimmed := strings.TrimSpace(string(expectedCF))
actualCFTrimmed := strings.TrimSpace(string(actualCF))
if actualCFTrimmed != expectedCFTrimmed {
diffString := diff.FormatDiff(expectedCFTrimmed, actualCFTrimmed)
t.Logf("diff:\n%s\n", diffString)

t.Fatalf("cloudformation output differed from expected")
Expand Down
4 changes: 3 additions & 1 deletion pkg/model/awsmodel/autoscalinggroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package awsmodel

import (
"fmt"

"github.com/golang/glog"

"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/model"
"k8s.io/kops/upup/pkg/fi"
Expand Down Expand Up @@ -117,7 +119,7 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
return err
}

if t.UserData, err = b.BootstrapScript.ResourceNodeUp(ig, b.Cluster.Spec.EgressProxy); err != nil {
if t.UserData, err = b.BootstrapScript.ResourceNodeUp(ig, &b.Cluster.Spec); err != nil {
return err
}

Expand Down
52 changes: 45 additions & 7 deletions pkg/model/bootstrapscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"strings"
"text/template"

"github.com/ghodss/yaml"

"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/nodeup"
"k8s.io/kops/pkg/model/resources"
Expand All @@ -37,7 +39,9 @@ type BootstrapScript struct {
NodeUpConfigBuilder func(ig *kops.InstanceGroup) (*nodeup.Config, error)
}

func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, ps *kops.EgressProxySpec) (*fi.ResourceHolder, error) {
// ResourceNodeUp generates and returns a nodeup (bootstrap) script from a
// template file, substituting in specific env vars & cluster spec configuration
func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, cs *kops.ClusterSpec) (*fi.ResourceHolder, error) {
if ig.Spec.Role == kops.InstanceGroupRoleBastion {
// Bastions are just bare machines (currently), used as SSH jump-hosts
return nil, nil
Expand Down Expand Up @@ -77,7 +81,7 @@ func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, ps *kops.Egress
},

"ProxyEnv": func() string {
return b.createProxyEnv(ps)
return b.createProxyEnv(cs.EgressProxy)
},
"AWS_REGION": func() string {
if os.Getenv("AWS_REGION") != "" {
Expand All @@ -86,6 +90,40 @@ func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, ps *kops.Egress
}
return ""
},

"ClusterSpec": func() (string, error) {
spec := make(map[string]interface{})
spec["cloudConfig"] = cs.CloudConfig
spec["docker"] = cs.Docker
spec["kubelet"] = cs.Kubelet
spec["kubeProxy"] = cs.KubeProxy

if ig.IsMaster() {
spec["kubeAPIServer"] = cs.KubeAPIServer
spec["kubeControllerManager"] = cs.KubeControllerManager
spec["kubeScheduler"] = cs.KubeScheduler
spec["masterKubelet"] = cs.MasterKubelet
}

content, err := yaml.Marshal(spec)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should actually use the typed version i.e. take the spec & zero out the bits we don't want, and then use the existing API marshalling. Then we could use this as the versioned API between kops & nodeup...

But I'm getting ahead of myself - this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good option. When I was using the existing API marshalling I was doing conversion from Spec -> JSON -> YAML, which looked a bit messy. Was a bit unsure on the most ideal approach.

if err != nil {
return "", fmt.Errorf("error converting cluster spec to yaml for inclusion within bootstrap script: %v", err)
}
return string(content), nil
},

"IGSpec": func() (string, error) {
spec := make(map[string]interface{})
spec["kubelet"] = ig.Spec.Kubelet
spec["nodeLabels"] = ig.Spec.NodeLabels
spec["taints"] = ig.Spec.Taints

content, err := yaml.Marshal(spec)
if err != nil {
return "", fmt.Errorf("error converting instancegroup spec to yaml for inclusion within bootstrap script: %v", err)
}
return string(content), nil
},
}

templateResource, err := NewTemplateResource("nodeup", resources.AWSNodeUpTemplate, functions, nil)
Expand All @@ -99,22 +137,22 @@ func (b *BootstrapScript) createProxyEnv(ps *kops.EgressProxySpec) string {
var buffer bytes.Buffer

if ps != nil && ps.HTTPProxy.Host != "" {
var httpProxyUrl string
var httpProxyURL string

// TODO double check that all the code does this
// TODO move this into a validate so we can enforce the string syntax
if !strings.HasPrefix(ps.HTTPProxy.Host, "http://") {
httpProxyUrl = "http://"
httpProxyURL = "http://"
}

if ps.HTTPProxy.Port != 0 {
httpProxyUrl += ps.HTTPProxy.Host + ":" + strconv.Itoa(ps.HTTPProxy.Port)
httpProxyURL += ps.HTTPProxy.Host + ":" + strconv.Itoa(ps.HTTPProxy.Port)
} else {
httpProxyUrl += ps.HTTPProxy.Host
httpProxyURL += ps.HTTPProxy.Host
}

// Set base env variables
buffer.WriteString("export http_proxy=" + httpProxyUrl + "\n")
buffer.WriteString("export http_proxy=" + httpProxyURL + "\n")
buffer.WriteString("export https_proxy=${http_proxy}\n")
buffer.WriteString("export no_proxy=" + ps.ProxyExcludes + "\n")
buffer.WriteString("export NO_PROXY=${no_proxy}\n")
Expand Down
130 changes: 130 additions & 0 deletions pkg/model/bootstrapscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ limitations under the License.
package model

import (
"io/ioutil"
"strings"
"testing"

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

func Test_ProxyFunc(t *testing.T) {
Expand Down Expand Up @@ -49,5 +51,133 @@ func Test_ProxyFunc(t *testing.T) {
if !strings.Contains(script, "export no_proxy="+ps.ProxyExcludes) {
t.Fatalf("script not setting no_proxy properly")
}
}

func TestBootstrapUserData(t *testing.T) {
cs := []struct {
Role kops.InstanceGroupRole
ExpectedFilePath string
}{
{
Role: "Master",
ExpectedFilePath: "tests/data/bootstrapscript_0.txt",
},
{
Role: "Node",
ExpectedFilePath: "tests/data/bootstrapscript_1.txt",
},
}

for i, x := range cs {
spec := makeTestCluster().Spec
group := makeTestInstanceGroup(x.Role)

renderNodeUpConfig := func(ig *kops.InstanceGroup) (*nodeup.Config, error) {
return &nodeup.Config{}, nil
}

bs := &BootstrapScript{
NodeUpSource: "NUSource",
NodeUpSourceHash: "NUSHash",
NodeUpConfigBuilder: renderNodeUpConfig,
}

res, err := bs.ResourceNodeUp(group, &spec)
if err != nil {
t.Errorf("case %d failed to create nodeup resource. error: %s", i, err)
continue
}

actual, err := res.AsString()
if err != nil {
t.Errorf("case %d failed to render nodeup resource. error: %s", i, err)
continue
}

expectedBytes, err := ioutil.ReadFile(x.ExpectedFilePath)
if err != nil {
t.Fatalf("unexpected error reading ExpectedFilePath %q: %v", x.ExpectedFilePath, err)
}

if actual != string(expectedBytes) {
t.Errorf("case %d, expected: %s. got: %s", i, string(expectedBytes), actual)
}
}
}

func makeTestCluster() *kops.Cluster {
return &kops.Cluster{
Spec: kops.ClusterSpec{
CloudProvider: "aws",
KubernetesVersion: "1.7.0",
Subnets: []kops.ClusterSubnetSpec{
{Name: "test", Zone: "eu-west-1a"},
},
NonMasqueradeCIDR: "10.100.0.0/16",
EtcdClusters: []*kops.EtcdClusterSpec{
{
Name: "main",
Members: []*kops.EtcdMemberSpec{
{
Name: "test",
InstanceGroup: s("ig-1"),
},
},
},
},
NetworkCIDR: "10.79.0.0/24",
CloudConfig: &kops.CloudConfiguration{
NodeTags: s("something"),
},
Docker: &kops.DockerConfig{
LogLevel: s("INFO"),
},
KubeAPIServer: &kops.KubeAPIServerConfig{
Image: "CoreOS",
},
KubeControllerManager: &kops.KubeControllerManagerConfig{
CloudProvider: "aws",
},
KubeProxy: &kops.KubeProxyConfig{
CPURequest: "30m",
FeatureGates: map[string]string{
"AdvancedAuditing": "true",
},
},
KubeScheduler: &kops.KubeSchedulerConfig{
Image: "SomeImage",
},
Kubelet: &kops.KubeletConfigSpec{
KubeconfigPath: "/etc/kubernetes/config.txt",
},
MasterKubelet: &kops.KubeletConfigSpec{
KubeconfigPath: "/etc/kubernetes/config.cfg",
},
EgressProxy: &kops.EgressProxySpec{
HTTPProxy: kops.HTTPProxy{
Host: "example.com",
Port: 80,
},
},
},
}
}

func makeTestInstanceGroup(role kops.InstanceGroupRole) *kops.InstanceGroup {
return &kops.InstanceGroup{
Spec: kops.InstanceGroupSpec{
Kubelet: &kops.KubeletConfigSpec{
KubeconfigPath: "/etc/kubernetes/igconfig.txt",
},
NodeLabels: map[string]string{
"labelname": "labelvalue",
"label2": "value2",
},
Role: role,
Taints: []string{
"key1=value1:NoSchedule",
"key2=value2:NoExecute",
},
},
}
}
3 changes: 2 additions & 1 deletion pkg/model/gcemodel/autoscalinggroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package gcemodel

import (
"fmt"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/model"
Expand All @@ -44,7 +45,7 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
for _, ig := range b.InstanceGroups {
name := b.SafeObjectName(ig.ObjectMeta.Name)

startupScript, err := b.BootstrapScript.ResourceNodeUp(ig, b.Cluster.Spec.EgressProxy)
startupScript, err := b.BootstrapScript.ResourceNodeUp(ig, &b.Cluster.Spec)
if err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/model/resources/nodeup.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ function download-release() {
echo "== nodeup node config starting =="
ensure-install-dir

cat > cluster_spec.yaml << __EOF_CLUSTER_SPEC
{{ ClusterSpec }}
__EOF_CLUSTER_SPEC

cat > ig_spec.yaml << __EOF_IG_SPEC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like where this is going :-)

{{ IGSpec }}
__EOF_IG_SPEC

cat > kube_env.yaml << __EOF_KUBE_ENV
{{ KubeEnv }}
__EOF_KUBE_ENV
Expand Down
Loading