This repository has been archived by the owner on May 22, 2020. It is now read-only.
Handling configurable machine setup/installation #664
+1,236
−430
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,8 +37,12 @@ import ( | |
|
|
||
| "regexp" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/client-go/kubernetes" | ||
| gceconfig "k8s.io/kube-deploy/cluster-api/cloud/google/gceproviderconfig" | ||
| gceconfigv1 "k8s.io/kube-deploy/cluster-api/cloud/google/gceproviderconfig/v1alpha1" | ||
| "k8s.io/kube-deploy/cluster-api/cloud/google/machinesetup" | ||
| apierrors "k8s.io/kube-deploy/cluster-api/errors" | ||
| clusterv1 "k8s.io/kube-deploy/cluster-api/pkg/apis/cluster/v1alpha1" | ||
| client "k8s.io/kube-deploy/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1" | ||
|
|
@@ -52,6 +56,8 @@ const ( | |
|
|
||
| UIDLabelKey = "machine-crd-uid" | ||
| BootstrapLabelKey = "boostrap" | ||
|
|
||
| MachineSetupConfigsFilename = "machine_setup_configs.yaml" | ||
|
There was a problem hiding this comment. Please add a comment about what this file contains and in what format. |
||
| ) | ||
|
|
||
| type SshCreds struct { | ||
|
|
@@ -66,14 +72,15 @@ type GCEClient struct { | |
| kubeadmToken string | ||
| sshCreds SshCreds | ||
| machineClient client.MachineInterface | ||
| configWatch *machinesetup.ConfigWatch | ||
| } | ||
|
|
||
| const ( | ||
| gceTimeout = time.Minute * 10 | ||
| gceWaitSleep = time.Second * 5 | ||
| ) | ||
|
|
||
| func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterface, configListPath string) (*GCEClient, error) { | ||
| // The default GCP client expects the environment variable | ||
| // GOOGLE_APPLICATION_CREDENTIALS to point to a file with service credentials. | ||
| client, err := google.DefaultClient(context.TODO(), compute.ComputeScope) | ||
|
|
@@ -104,6 +111,15 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa | |
| } | ||
| } | ||
|
|
||
| // TODO: get rid of empty string check when we switch to the new bootstrapping method. | ||
| var configWatch *machinesetup.ConfigWatch | ||
| if configListPath != "" { | ||
|
There was a problem hiding this comment. I don't think this check is necessary. There was a problem hiding this comment. Create is the only command that has to pass a config list path, and then add and delete pass an empty string to the method. I put this check here to handle the empty string from add and delete, and create has a check to prevent an empty string. NewConfigWatch() checks that the path is valid. I can move the empty string check to NewConfigWatch() so it just returns nil (but no error) if it gets an empty string if that makes more sense. There was a problem hiding this comment. Fair enough. Can we add TODOs to remove this when we switch to the new bootstrapping method. In that setup, we will be running a full-blown controller that will always need a config path. There was a problem hiding this comment. So would add/delete also need to pass the config path when we switch over (and I should add TODOs there too)? |
||
| configWatch, err = machinesetup.NewConfigWatch(configListPath) | ||
| if err != nil { | ||
| glog.Errorf("Error creating config watch: %v", err) | ||
| } | ||
| } | ||
|
|
||
| return &GCEClient{ | ||
| service: service, | ||
| scheme: scheme, | ||
|
|
@@ -114,10 +130,11 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa | |
| user: user, | ||
| }, | ||
| machineClient: machineClient, | ||
| configWatch: configWatch, | ||
| }, nil | ||
| } | ||
|
|
||
| func (gce *GCEClient) CreateMachineController(cluster *clusterv1.Cluster, initialMachines []*clusterv1.Machine, clientSet kubernetes.Clientset) error { | ||
| if err := gce.CreateMachineControllerServiceAccount(cluster, initialMachines); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -131,6 +148,26 @@ func (gce *GCEClient) CreateMachineController(cluster *clusterv1.Cluster, initia | |
| return err | ||
| } | ||
|
|
||
| // Create the configmap so the machine setup configs can be mounted into the node. | ||
| machineSetupConfigs, err := gce.configWatch.ValidConfigs() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| yaml, err := machineSetupConfigs.GetYaml() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| configMap := corev1.ConfigMap{ | ||
| ObjectMeta: v1.ObjectMeta{Name: "machine-setup"}, | ||
| Data: map[string]string{ | ||
| MachineSetupConfigsFilename: yaml, | ||
| }, | ||
| } | ||
| configMaps := clientSet.CoreV1().ConfigMaps(corev1.NamespaceDefault) | ||
| if _, err := configMaps.Create(&configMap); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := CreateApiServerAndController(gce.kubeadmToken); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -153,22 +190,32 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach | |
| return errors.New("invalid master configuration: missing Machine.Spec.Versions.Kubelet") | ||
| } | ||
|
|
||
| machineSetupConfigs, err := gce.configWatch.ValidConfigs() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| configParams := &machinesetup.ConfigParams{ | ||
| OS: config.OS, | ||
| Roles: machine.Spec.Roles, | ||
| Versions: machine.Spec.Versions, | ||
| } | ||
| image, err := machineSetupConfigs.GetImage(configParams) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| imagePath := gce.getImagePath(image) | ||
|
|
||
| machineSetupMetadata, err := machineSetupConfigs.GetMetadata(configParams) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if util.IsMaster(machine) { | ||
| if machine.Spec.Versions.ControlPlane == "" { | ||
| return gce.handleMachineError(machine, apierrors.InvalidMachineConfiguration( | ||
| "invalid master configuration: missing Machine.Spec.Versions.ControlPlane")) | ||
| } | ||
| var err error | ||
| metadata, err = masterMetadata(gce.kubeadmToken, cluster, machine, config.Project, &machineSetupMetadata) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -177,14 +224,7 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach | |
| return errors.New("invalid cluster state: cannot create a Kubernetes node without an API endpoint") | ||
| } | ||
| var err error | ||
| metadata, err = nodeMetadata(gce.kubeadmToken, cluster, machine, &machineSetupMetadata) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -207,13 +247,7 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach | |
| name := machine.ObjectMeta.Name | ||
| project := config.Project | ||
| zone := config.Zone | ||
| diskSize := int64(30) | ||
|
|
||
| if instance == nil { | ||
| labels := map[string]string{ | ||
|
|
@@ -242,7 +276,7 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach | |
| AutoDelete: true, | ||
| Boot: true, | ||
| InitializeParams: &compute.AttachedDiskInitializeParams{ | ||
| SourceImage: imagePath, | ||
| DiskSizeGb: diskSize, | ||
| }, | ||
| }, | ||
|
|
@@ -653,42 +687,29 @@ func (gce *GCEClient) handleMachineError(machine *clusterv1.Machine, err *apierr | |
| return err | ||
| } | ||
|
|
||
| func (gce *GCEClient) getImagePath(img string) (imagePath string) { | ||
|
There was a problem hiding this comment. This is a great method for testing with unit tests. There was a problem hiding this comment. I agree, but it makes a call to the GCE API to check that the image is there and I was having trouble figuring out how to mock it (otherwise I would have added the test when I refactored this method in a previous PR). Rob is working on a PR (#688) to set up a fake client so I can write a unit test for this method once that gets merged. |
||
| defaultImg := "projects/ubuntu-os-cloud/global/images/family/ubuntu-1710" | ||
|
|
||
| // A full image path must match the regex format. If it doesn't, we will fall back to a default base image. | ||
| matches := regexp.MustCompile("projects/(.+)/global/images/(family/)*(.+)").FindStringSubmatch(img) | ||
| if matches != nil { | ||
| // Check to see if the image exists in the given path. The presence of "family" in the path dictates which API call we need to make. | ||
| project, family, name := matches[1], matches[2], matches[3] | ||
| var err error | ||
| if family == "" { | ||
| _, err = gce.service.Images.Get(project, name).Do() | ||
| } else { | ||
| _, err = gce.service.Images.GetFromFamily(project, name).Do() | ||
| } | ||
|
|
||
| if err == nil { | ||
| return img | ||
| } | ||
| } | ||
|
|
||
| // Otherwise, fall back to the base image. | ||
| glog.Infof("Could not find image at %s. Defaulting to %s.", img, defaultImg) | ||
| return defaultImg | ||
| } | ||
|
|
||
| // Just a temporary hack to grab a single range from the config. | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call it this something other than
configplease. There are way too many things namedconfig. Be specific here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I named it machinesetup to be consistent with the flag in create.go.