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

v1alpha2 API #1183

Merged
merged 23 commits into from Dec 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions Makefile
Expand Up @@ -244,3 +244,15 @@ channels-gocode:

examples:
go install k8s.io/kops/examples/kops-api-example/...

# -----------------------------------------------------
# api machinery regenerate

apimachinery:
#go install ./cmd/libs/go2idl/conversion-gen
~/k8s/bin/conversion-gen --input-dirs k8s.io/kops/pkg/apis/kops/v1alpha1 --v=8 --output-file-base=zz_generated.conversion
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have the comments parts in another target and have this documented?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well... so the challenge is that the fix I had to make it k8s so the conversion generator works only just landed today. So we also have to update k8s to the latest, or pull it out into the shared repo. I'm inclined to do the latter. It's a placeholder for actually installing the code for real

Copy link
Contributor

Choose a reason for hiding this comment

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

I would encourage us to bump k8s version before this merges.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just starting review but do we have an API machinery doc floating around this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't but we have #1164

Bumping k8s version means going onto the master branch :-(

#go install github.com/ugorji/go/codec/codecgen
# codecgen works only if invoked from directory where the file is located.
#cd pkg/apis/kops/v1alpha2/ && ~/k8s/bin/codecgen -d 1234 -o types.generated.go instancegroup.go cluster.go federation.go
#cd pkg/apis/kops/v1alpha1/ && ~/k8s/bin/codecgen -d 1234 -o types.generated.go instancegroup.go cluster.go federation.go
#cd pkg/apis/kops/ && ~/k8s/bin/codecgen -d 1234 -o types.generated.go instancegroup.go cluster.go federation.go
2 changes: 1 addition & 1 deletion _vendor/github.com/aws/aws-sdk-go
Submodule aws-sdk-go updated 680 files
2 changes: 1 addition & 1 deletion _vendor/k8s.io/kubernetes
Submodule kubernetes updated 6229 files
24 changes: 24 additions & 0 deletions cloudmock/aws/mockec2/convenience.go
@@ -0,0 +1,24 @@
/*
Copyright 2016 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 mockec2

import "github.com/aws/aws-sdk-go/aws"

// s is a helper that builds a *string from a string value
func s(v string) *string {
return aws.String(v)
}
141 changes: 97 additions & 44 deletions cmd/kops/create_cluster.go
Expand Up @@ -33,7 +33,7 @@ import (
"k8s.io/kops/upup/pkg/fi/cloudup"
"k8s.io/kops/upup/pkg/fi/utils"
"k8s.io/kops/upup/pkg/kutil"
"k8s.io/kubernetes/pkg/util/sets"
"sort"
)

type CreateClusterOptions struct {
Expand Down Expand Up @@ -82,6 +82,7 @@ func (o *CreateClusterOptions) InitDefaults() {

func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
options := &CreateClusterOptions{}
options.InitDefaults()

cmd := &cobra.Command{
Use: "cluster",
Expand Down Expand Up @@ -229,20 +230,24 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
glog.V(4).Infof("networking mode=%s => %s", c.Networking, fi.DebugAsJsonString(cluster.Spec.Networking))

if c.Zones != "" {
existingZones := make(map[string]*api.ClusterZoneSpec)
for _, zone := range cluster.Spec.Zones {
existingZones[zone.Name] = zone
existingSubnets := make(map[string]*api.ClusterSubnetSpec)
for i := range cluster.Spec.Subnets {
subnet := &cluster.Spec.Subnets[i]
existingSubnets[subnet.Name] = subnet
}
for _, zone := range parseZoneList(c.Zones) {
if existingZones[zone] == nil {
cluster.Spec.Zones = append(cluster.Spec.Zones, &api.ClusterZoneSpec{
Name: zone,
for _, zoneName := range parseZoneList(c.Zones) {
// We create default subnets named the same as the zones
subnetName := zoneName
if existingSubnets[subnetName] == nil {
cluster.Spec.Subnets = append(cluster.Spec.Subnets, api.ClusterSubnetSpec{
Name: subnetName,
Zone: subnetName,
})
}
}
}

if len(cluster.Spec.Zones) == 0 {
if len(cluster.Spec.Subnets) == 0 {
return fmt.Errorf("must specify at least one zone for the cluster (use --zones)")
}

Expand All @@ -255,13 +260,13 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
// We default to single-master (not HA), unless the user explicitly specifies it
// HA master is a little slower, not as well tested yet, and requires more resources
// Probably best not to make it the silent default!
for _, zone := range cluster.Spec.Zones {
for _, subnet := range cluster.Spec.Subnets {
g := &api.InstanceGroup{}
g.Spec.Role = api.InstanceGroupRoleMaster
g.Spec.Zones = []string{zone.Name}
g.Spec.Subnets = []string{subnet.Name}
g.Spec.MinSize = fi.Int(1)
g.Spec.MaxSize = fi.Int(1)
g.ObjectMeta.Name = "master-" + zone.Name // Subsequent masters (if we support that) could be <zone>-1, <zone>-2
g.ObjectMeta.Name = "master-" + subnet.Name // Subsequent masters (if we support that) could be <zone>-1, <zone>-2
instanceGroups = append(instanceGroups, g)
masters = append(masters, g)

Expand All @@ -272,13 +277,13 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
} else {
if len(masters) == 0 {
// Use the specified master zones (this is how the user gets HA master)
for _, zone := range parseZoneList(c.MasterZones) {
for _, subnetName := range parseZoneList(c.MasterZones) {
g := &api.InstanceGroup{}
g.Spec.Role = api.InstanceGroupRoleMaster
g.Spec.Zones = []string{zone}
g.Spec.Subnets = []string{subnetName}
g.Spec.MinSize = fi.Int(1)
g.Spec.MaxSize = fi.Int(1)
g.ObjectMeta.Name = "master-" + zone
g.ObjectMeta.Name = "master-" + subnetName
instanceGroups = append(instanceGroups, g)
masters = append(masters, g)
}
Expand All @@ -289,21 +294,44 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}

if len(cluster.Spec.EtcdClusters) == 0 {
zones := sets.NewString()
for _, group := range masters {
for _, zone := range group.Spec.Zones {
zones.Insert(zone)
subnetMap := make(map[string]*api.ClusterSubnetSpec)
for i := range cluster.Spec.Subnets {
subnet := &cluster.Spec.Subnets[i]
subnetMap[subnet.Name] = subnet
}

var masterNames []string
masterInstanceGroups := make(map[string]*api.InstanceGroup)
for _, ig := range masters {
if len(ig.Spec.Subnets) != 1 {
return fmt.Errorf("unexpected subnets for master instance group %q (expected exactly only, found %d)", ig.ObjectMeta.Name, len(ig.Spec.Subnets))
}
masterNames = append(masterNames, ig.Spec.Subnets[0])

for _, subnetName := range ig.Spec.Subnets {
subnet := subnetMap[subnetName]
if subnet == nil {
return fmt.Errorf("cannot find subnet %q (declared in instance group %q, not found in cluster)", subnetName, ig.ObjectMeta.Name)
}

if masterInstanceGroups[subnetName] != nil {
return fmt.Errorf("found two master instance groups in subnet %q", subnetName)
}

masterInstanceGroups[subnetName] = ig
}
}
etcdZones := zones.List()

sort.Strings(masterNames)

for _, etcdCluster := range cloudup.EtcdClusters {
etcd := &api.EtcdClusterSpec{}
etcd.Name = etcdCluster
for _, zone := range etcdZones {
for _, masterName := range masterNames {
ig := masterInstanceGroups[masterName]
m := &api.EtcdMemberSpec{}
m.Name = zone
m.Zone = fi.String(zone)
m.Name = ig.ObjectMeta.Name
m.InstanceGroup = fi.String(ig.ObjectMeta.Name)
etcd.Members = append(etcd.Members, m)
}
cluster.Spec.EtcdClusters = append(cluster.Spec.EtcdClusters, etcd)
Expand Down Expand Up @@ -380,10 +408,10 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}

if cluster.Spec.CloudProvider == "" {
for _, zone := range cluster.Spec.Zones {
cloud, known := fi.GuessCloudForZone(zone.Name)
for _, subnet := range cluster.Spec.Subnets {
cloud, known := fi.GuessCloudForZone(subnet.Zone)
if known {
glog.Infof("Inferred --cloud=%s from zone %q", cloud, zone.Name)
glog.Infof("Inferred --cloud=%s from zone %q", cloud, subnet.Zone)
cluster.Spec.CloudProvider = string(cloud)
break
}
Expand All @@ -393,21 +421,29 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}
}

//Bastion
if c.Topology != "" {
if c.Topology == api.TopologyPublic && c.Bastion == true {
return fmt.Errorf("Bastion supports --topology='private' only.")
}
// Network Topology
if c.Topology == "" {
// The flag default should have set this, but we might be being called as a library
glog.Infof("Empty topology. Defaulting to public topology")
c.Topology = api.TopologyPublic
}

// Network Topology
switch c.Topology {
case api.TopologyPublic:
cluster.Spec.Topology = &api.TopologySpec{
Masters: api.TopologyPublic,
Nodes: api.TopologyPublic,
Bastion: &api.BastionSpec{Enable: c.Bastion},
//Bastion: &api.BastionSpec{Enable: c.Bastion},
}

if c.Bastion {
return fmt.Errorf("Bastion supports --topology='private' only.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of the command line flag connotations in error messages?

We call kops as a library often, and the users typically have no concept of --topology

Wordsmith:

return fmt.Errorf("Attempting to create bastion, in a non-bastion configuration (Topology: private)")

}

for i := range cluster.Spec.Subnets {
cluster.Spec.Subnets[i].Type = api.SubnetTypePublic
}

case api.TopologyPrivate:
if !supportsPrivateTopology(cluster.Spec.Networking) {
return fmt.Errorf("Invalid networking option %s. Currently only '--networking cni', '--networking kopeio-vxlan', '--networking weave' are supported for private topologies", c.Networking)
Expand All @@ -416,19 +452,35 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
Masters: api.TopologyPrivate,
Nodes: api.TopologyPrivate,
}
cluster.Spec.Topology.Bastion = &api.BastionSpec{Enable: c.Bastion}
case "":
glog.Warningf("Empty topology. Defaulting to public topology without bastion")
cluster.Spec.Topology = &api.TopologySpec{
Masters: api.TopologyPublic,
Nodes: api.TopologyPublic,
Bastion: &api.BastionSpec{Enable: false},

for i := range cluster.Spec.Subnets {
cluster.Spec.Subnets[i].Type = api.SubnetTypePrivate
}

var utilitySubnets []api.ClusterSubnetSpec
for _, s := range cluster.Spec.Subnets {
if s.Type == api.SubnetTypeUtility {
continue
}
subnet := api.ClusterSubnetSpec{
Name: "utility-" + s.Name,
Zone: s.Zone,
Type: api.SubnetTypeUtility,
}
utilitySubnets = append(utilitySubnets, subnet)
}
cluster.Spec.Subnets = append(cluster.Spec.Subnets, utilitySubnets...)

if c.Bastion {
bastionGroup := &api.InstanceGroup{}
bastionGroup.Spec.Role = api.InstanceGroupRoleBastion
bastionGroup.ObjectMeta.Name = "bastions"
instanceGroups = append(instanceGroups, bastionGroup)
}

default:
return fmt.Errorf("Invalid topology %s.", c.Topology)
}
cluster.Spec.Topology.Bastion.MachineType = cloudup.DefaultBastionMachineType(cluster)
cluster.Spec.Topology.Bastion.IdleTimeout = cloudup.DefaultBastionIdleTimeout(cluster)

sshPublicKeys := make(map[string][]byte)
if c.SSHPublicKey != "" {
Expand All @@ -443,10 +495,11 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}

if c.AdminAccess != "" {
cluster.Spec.AdminAccess = []string{c.AdminAccess}
cluster.Spec.SSHAccess = []string{c.AdminAccess}
cluster.Spec.KubernetesAPIAccess = []string{c.AdminAccess}
}

err = cluster.PerformAssignments()
err = cloudup.PerformAssignments(cluster)
if err != nil {
return fmt.Errorf("error populating configuration: %v", err)
}
Expand Down
14 changes: 8 additions & 6 deletions cmd/kops/create_cluster_integration_test.go
Expand Up @@ -35,19 +35,21 @@ var MagicTimestamp = unversioned.Time{Time: time.Date(2017, 1, 1, 0, 0, 0, 0, ti

// TestCreateClusterMinimal runs kops create cluster minimal.example.com --zones us-test-1a
func TestCreateClusterMinimal(t *testing.T) {
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/minimal")
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/minimal", "v1alpha1")
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/minimal", "v1alpha2")
}

// TestCreateClusterHA runs kops create cluster ha.example.com --zones us-test-1a,us-test-1b,us-test-1c --master-zones us-test-1a,us-test-1b,us-test-1c
func TestCreateClusterHA(t *testing.T) {
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha")
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha", "v1alpha1")
runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/ha", "v1alpha2")
}

func runCreateClusterIntegrationTest(t *testing.T, srcDir string) {
func runCreateClusterIntegrationTest(t *testing.T, srcDir string, version string) {
var stdout bytes.Buffer

optionsYAML := "options.yaml"
expectedClusterPath := "expected.yaml"
expectedClusterPath := "expected-" + version + ".yaml"

factoryOptions := &util.FactoryOptions{}
factoryOptions.RegistryPath = "memfs://tests"
Expand Down Expand Up @@ -112,7 +114,7 @@ func runCreateClusterIntegrationTest(t *testing.T, srcDir string) {

for _, cluster := range clusters.Items {
cluster.ObjectMeta.CreationTimestamp = MagicTimestamp
actualYAMLBytes, err := kops.ToVersionedYaml(&cluster)
actualYAMLBytes, err := kops.ToVersionedYamlWithVersion(&cluster, version)
if err != nil {
t.Fatalf("unexpected error serializing cluster: %v", err)
}
Expand All @@ -131,7 +133,7 @@ func runCreateClusterIntegrationTest(t *testing.T, srcDir string) {
for _, ig := range instanceGroups.Items {
ig.ObjectMeta.CreationTimestamp = MagicTimestamp

actualYAMLBytes, err := kops.ToVersionedYaml(&ig)
actualYAMLBytes, err := kops.ToVersionedYamlWithVersion(&ig, version)
if err != nil {
t.Fatalf("unexpected error serializing InstanceGroup: %v", err)
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/kops/create_ig.go
Expand Up @@ -96,7 +96,12 @@ func RunCreateInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string,
// Populate some defaults
ig := &api.InstanceGroup{}
ig.ObjectMeta.Name = groupName
ig.Spec.Role = api.InstanceGroupRole(options.Role)

role, ok := api.ParseInstanceGroupRole(options.Role, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

So I got coached on using err, so we should use err ;)

Copy link
Contributor

@krisnova krisnova Dec 19, 2016

Choose a reason for hiding this comment

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

Another case of weird golangisms...

This is actually fine - the function api.ParseInstanceGroupRole() is not returning an err but rather returning a bool than be used in a very similar way as err

Usually you see this if the function doesn't actually do anything wrong, just returns a bool that the user can trigger off of

if !ok {
return fmt.Errorf("unknown role %q", options.Role)
}
ig.Spec.Role = role

ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, channel)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_cluster.go
Expand Up @@ -118,7 +118,7 @@ func RunEditCluster(f *util.Factory, cmd *cobra.Command, args []string, out io.W
if !ok {
return fmt.Errorf("object was not of expected type: %T", newObj)
}
err = newCluster.PerformAssignments()
err = cloudup.PerformAssignments(newCluster)
if err != nil {
return fmt.Errorf("error populating configuration: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_instancegroup.go
Expand Up @@ -138,7 +138,7 @@ func RunEditInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string, ou

// We need the full cluster spec to perform deep validation
// Note that we don't write it back though
err = cluster.PerformAssignments()
err = cloudup.PerformAssignments(cluster)
if err != nil {
return fmt.Errorf("error populating configuration: %v", err)
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/kops/get_cluster.go
Expand Up @@ -109,14 +109,14 @@ func (c *GetClustersCmd) Run(args []string) error {
t.AddColumn("CLOUD", func(c *api.Cluster) string {
return c.Spec.CloudProvider
})
t.AddColumn("ZONES", func(c *api.Cluster) string {
var zoneNames []string
for _, z := range c.Spec.Zones {
zoneNames = append(zoneNames, z.Name)
t.AddColumn("SUBNETS", func(c *api.Cluster) string {
var subnetNames []string
for _, s := range c.Spec.Subnets {
subnetNames = append(subnetNames, s.Name)
}
return strings.Join(zoneNames, ",")
return strings.Join(subnetNames, ",")
})
return t.Render(clusters, os.Stdout, "NAME", "CLOUD", "ZONES")
return t.Render(clusters, os.Stdout, "NAME", "CLOUD", "SUBNETS")

case OutputYaml:
for _, cluster := range clusters {
Expand Down