Skip to content

Commit

Permalink
Merge pull request #8159 from johngmyers/validate-spot
Browse files Browse the repository at this point in the history 
Always consider spot instance node readiness in cluster validation
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Dec 27, 2019
2 parents dd608e8 + 80dc001 commit 8436a3e
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 28 deletions.
1 change: 0 additions & 1 deletion pkg/validation/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/apis/kops:go_default_library",
"//pkg/apis/kops/util:go_default_library",
"//pkg/cloudinstances:go_default_library",
"//pkg/dns:go_default_library",
"//upup/pkg/fi:go_default_library",
Expand Down
4 changes: 1 addition & 3 deletions pkg/validation/validate_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/client-go/tools/clientcmd"
"k8s.io/klog"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/util"
"k8s.io/kops/pkg/cloudinstances"
"k8s.io/kops/pkg/dns"
)
Expand Down Expand Up @@ -267,7 +266,7 @@ func (v *ValidationCluster) validateNodes(cloudGroups map[string]*cloudinstances
continue
}

role := util.GetNodeRole(node)
role := strings.ToLower(string(cloudGroup.InstanceGroup.Spec.Role))
if role == "" {
role = "node"
}
Expand All @@ -282,7 +281,6 @@ func (v *ValidationCluster) validateNodes(cloudGroups map[string]*cloudinstances

ready := isNodeReady(node)

// TODO: Use instance group role instead...
if n.Role == "master" {
if !ready {
v.addError(&ValidationError{
Expand Down
189 changes: 165 additions & 24 deletions pkg/validation/validate_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,59 @@ func Test_ValidateNodesNotEnough(t *testing.T) {
Role: kopsapi.InstanceGroupRoleNode,
},
},
MinSize: 3,
Ready: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00001",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "node-1a"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionTrue},
},
},
},
},
},
NeedUpdate: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00002",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "node-1b"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionTrue},
},
},
},
},
},
}

v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 1) ||
!assert.Equal(t, &ValidationError{
Kind: "InstanceGroup",
Name: "node-1",
Message: "InstanceGroup \"node-1\" did not have enough nodes 2 vs 3",
}, v.Failures[0]) {
printDebug(t, v)
}
}

func Test_ValidateNodeNotReady(t *testing.T) {
groups := make(map[string]*cloudinstances.CloudInstanceGroup)
groups["node-1"] = &cloudinstances.CloudInstanceGroup{
InstanceGroup: &kopsapi.InstanceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "node-1",
},
Spec: kopsapi.InstanceGroupSpec{
Role: kopsapi.InstanceGroupRoleNode,
},
},
MinSize: 2,
Ready: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00001",
Expand Down Expand Up @@ -151,32 +204,120 @@ func Test_ValidateNodesNotEnough(t *testing.T) {
},
}

t.Run("too few nodes", func(t *testing.T) {
groups["node-1"].MinSize = 3
v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 2) {
printDebug(t, v)
}
})
v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 1) ||
!assert.Equal(t, &ValidationError{
Kind: "Node",
Name: "node-1b",
Message: "node \"node-1b\" is not ready",
}, v.Failures[0]) {
printDebug(t, v)
}
}

t.Run("not ready node", func(t *testing.T) {
groups["node-1"].MinSize = 2
v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 1) {
printDebug(t, v)
}
})
func Test_ValidateMastersNotEnough(t *testing.T) {
groups := make(map[string]*cloudinstances.CloudInstanceGroup)
groups["node-1"] = &cloudinstances.CloudInstanceGroup{
InstanceGroup: &kopsapi.InstanceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "master-1",
},
Spec: kopsapi.InstanceGroupSpec{
Role: kopsapi.InstanceGroupRoleMaster,
},
},
MinSize: 3,
Ready: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00001",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "master-1a"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionTrue},
},
},
},
},
},
NeedUpdate: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00002",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "master-1b"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionTrue},
},
},
},
},
},
}

t.Run("unexpected errors", func(t *testing.T) {
groups["node-1"].NeedUpdate[0].Node.Status.Conditions[0].Status = v1.ConditionTrue
v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Empty(t, v.Failures) {
printDebug(t, v)
}
})
v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 1) ||
!assert.Equal(t, &ValidationError{
Kind: "InstanceGroup",
Name: "master-1",
Message: "InstanceGroup \"master-1\" did not have enough nodes 2 vs 3",
}, v.Failures[0]) {
printDebug(t, v)
}
}

func Test_ValidateMasterNotReady(t *testing.T) {
groups := make(map[string]*cloudinstances.CloudInstanceGroup)
groups["node-1"] = &cloudinstances.CloudInstanceGroup{
InstanceGroup: &kopsapi.InstanceGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "master-1",
},
Spec: kopsapi.InstanceGroupSpec{
Role: kopsapi.InstanceGroupRoleMaster,
},
},
MinSize: 2,
Ready: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00001",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "master-1a"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionTrue},
},
},
},
},
},
NeedUpdate: []*cloudinstances.CloudInstanceGroupMember{
{
ID: "i-00002",
Node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: "master-1b"},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: "Ready", Status: v1.ConditionFalse},
},
},
},
},
},
}

v, err := testValidate(t, groups, nil)
require.NoError(t, err)
if !assert.Len(t, v.Failures, 1) ||
!assert.Equal(t, &ValidationError{
Kind: "Node",
Name: "master-1b",
Message: "master \"master-1b\" is not ready",
}, v.Failures[0]) {
printDebug(t, v)
}
}

func Test_ValidateNoComponentFailures(t *testing.T) {
Expand Down

0 comments on commit 8436a3e

Please sign in to comment.