diff --git a/cloudmock/openstack/mockcompute/servers.go b/cloudmock/openstack/mockcompute/servers.go index f2127f7b92b92..2a0b21cb8d7fe 100644 --- a/cloudmock/openstack/mockcompute/servers.go +++ b/cloudmock/openstack/mockcompute/servers.go @@ -173,7 +173,7 @@ func (m *MockClient) listServers(w http.ResponseWriter, vals url.Values) { func (m *MockClient) deleteServer(w http.ResponseWriter, serverID string) { if _, ok := m.servers[serverID]; ok { delete(m.servers, serverID) - w.WriteHeader(http.StatusOK) + w.WriteHeader(http.StatusNoContent) } else { w.WriteHeader(http.StatusNotFound) } @@ -186,6 +186,11 @@ func (m *MockClient) createServer(w http.ResponseWriter, r *http.Request) { panic("error decoding create server request") } + if len(create.Server.Networks) == 0 { + w.WriteHeader(http.StatusBadRequest) + return + } + w.WriteHeader(http.StatusCreated) server := servers.Server{ diff --git a/cloudmock/openstack/mocknetworking/ports.go b/cloudmock/openstack/mocknetworking/ports.go index 3de7abe5ed39c..a0f0e7c0983bf 100644 --- a/cloudmock/openstack/mocknetworking/ports.go +++ b/cloudmock/openstack/mocknetworking/ports.go @@ -172,13 +172,16 @@ func (m *MockClient) createPort(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusAccepted) p := ports.Port{ - ID: uuid.New().String(), - Name: create.Port.Name, - NetworkID: create.Port.NetworkID, - SecurityGroups: *create.Port.SecurityGroups, - DeviceID: create.Port.DeviceID, - FixedIPs: fixedIPs, + ID: uuid.New().String(), + Name: create.Port.Name, + NetworkID: create.Port.NetworkID, + DeviceID: create.Port.DeviceID, + FixedIPs: fixedIPs, } + if create.Port.SecurityGroups != nil { + p.SecurityGroups = *create.Port.SecurityGroups + } + m.ports[p.ID] = p resp := portGetResponse{ diff --git a/cmd/kops/lifecycle_integration_test.go b/cmd/kops/lifecycle_integration_test.go index 849102204811b..e82aeb755c0a6 100644 --- a/cmd/kops/lifecycle_integration_test.go +++ b/cmd/kops/lifecycle_integration_test.go @@ -336,7 +336,7 @@ func runLifecycleTestOpenstack(o *LifecycleTestOptions) { }() h.MockKopsVersion("1.19.0-alpha.3") - cloud := h.SetupMockOpenstack() + cloud := testutils.SetupMockOpenstack() var beforeIds []string for id := range AllOpenstackResources(cloud) { diff --git a/cmd/kops/rollingupdatecluster.go b/cmd/kops/rollingupdatecluster.go index a5dbf0a75f987..203d059b8c1e0 100644 --- a/cmd/kops/rollingupdatecluster.go +++ b/cmd/kops/rollingupdatecluster.go @@ -328,6 +328,7 @@ func RunRollingUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer } d := &instancegroups.RollingUpdateCluster{ + Clientset: clientset, Ctx: ctx, Cluster: cluster, MasterInterval: options.MasterInterval, diff --git a/pkg/instancegroups/BUILD.bazel b/pkg/instancegroups/BUILD.bazel index be31d811c6dff..022cd0143690d 100644 --- a/pkg/instancegroups/BUILD.bazel +++ b/pkg/instancegroups/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//pkg/featureflag:go_default_library", "//pkg/validation:go_default_library", "//upup/pkg/fi:go_default_library", + "//upup/pkg/fi/cloudup:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", @@ -33,6 +34,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "rollingupdate_os_test.go", "rollingupdate_test.go", "settings_test.go", ], @@ -40,15 +42,23 @@ go_test( deps = [ "//cloudmock/aws/mockautoscaling:go_default_library", "//pkg/apis/kops:go_default_library", + "//pkg/assets:go_default_library", + "//pkg/client/simple/vfsclientset:go_default_library", "//pkg/cloudinstances:go_default_library", + "//pkg/testutils:go_default_library", "//pkg/validation:go_default_library", "//upup/pkg/fi:go_default_library", + "//upup/pkg/fi/cloudup:go_default_library", "//upup/pkg/fi/cloudup/awsup:go_default_library", + "//upup/pkg/fi/cloudup/openstack:go_default_library", + "//util/pkg/vfs:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/autoscaling:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/ec2/ec2iface:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack/compute/v2/servers:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/ports:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/instancegroups/instancegroups.go b/pkg/instancegroups/instancegroups.go index 9ed5b4c0e03ad..bc78a8acbf591 100644 --- a/pkg/instancegroups/instancegroups.go +++ b/pkg/instancegroups/instancegroups.go @@ -24,6 +24,9 @@ import ( "strings" "time" + "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/upup/pkg/fi/cloudup" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -371,6 +374,11 @@ func (c *RollingUpdateCluster) drainTerminateAndWait(u *cloudinstances.CloudInst return err } + if err := c.reconcileInstanceGroup(); err != nil { + klog.Errorf("error reconciling instance group %q: %v", u.CloudInstanceGroup.HumanName, err) + return err + } + // Wait for the minimum interval klog.Infof("waiting for %v after terminating instance", sleepAfterTerminate) time.Sleep(sleepAfterTerminate) @@ -378,6 +386,29 @@ func (c *RollingUpdateCluster) drainTerminateAndWait(u *cloudinstances.CloudInst return nil } +func (c *RollingUpdateCluster) reconcileInstanceGroup() error { + if api.CloudProviderID(c.Cluster.Spec.CloudProvider) != api.CloudProviderOpenstack { + return nil + } + rto := fi.RunTasksOptions{} + rto.InitDefaults() + applyCmd := &cloudup.ApplyClusterCmd{ + Cloud: c.Cloud, + Clientset: c.Clientset, + Cluster: c.Cluster, + DryRun: false, + AllowKopsDowngrade: true, + RunTasksOptions: &rto, + OutDir: "", + Phase: "", + TargetName: "direct", + LifecycleOverrides: map[string]fi.Lifecycle{}, + } + + return applyCmd.Run(c.Ctx) + +} + func (c *RollingUpdateCluster) maybeValidate(operation string, validateCount int) error { if c.CloudOnly { klog.Warningf("Not validating cluster as cloudonly flag is set.") diff --git a/pkg/instancegroups/rollingupdate.go b/pkg/instancegroups/rollingupdate.go index 178e4a696cd3b..70a6366ad6352 100644 --- a/pkg/instancegroups/rollingupdate.go +++ b/pkg/instancegroups/rollingupdate.go @@ -23,6 +23,8 @@ import ( "sync" "time" + "k8s.io/kops/pkg/client/simple" + "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" api "k8s.io/kops/pkg/apis/kops" @@ -33,9 +35,10 @@ import ( // RollingUpdateCluster is a struct containing cluster information for a rolling update. type RollingUpdateCluster struct { - Ctx context.Context - Cluster *api.Cluster - Cloud fi.Cloud + Clientset simple.Clientset + Ctx context.Context + Cluster *api.Cluster + Cloud fi.Cloud // MasterInterval is the amount of time to wait after stopping a master instance MasterInterval time.Duration diff --git a/pkg/instancegroups/rollingupdate_os_test.go b/pkg/instancegroups/rollingupdate_os_test.go new file mode 100644 index 0000000000000..d52e64575c8e9 --- /dev/null +++ b/pkg/instancegroups/rollingupdate_os_test.go @@ -0,0 +1,213 @@ +/* +Copyright 2020 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 instancegroups + +import ( + "context" + "os" + "testing" + "time" + + "k8s.io/kops/upup/pkg/fi" + + "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" + + "k8s.io/kops/util/pkg/vfs" + + "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + v1meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + kopsapi "k8s.io/kops/pkg/apis/kops" + "k8s.io/kops/pkg/assets" + "k8s.io/kops/pkg/client/simple/vfsclientset" + "k8s.io/kops/pkg/cloudinstances" + "k8s.io/kops/pkg/testutils" + "k8s.io/kops/upup/pkg/fi/cloudup" + "k8s.io/kops/upup/pkg/fi/cloudup/openstack" +) + +func getTestSetupOS(t *testing.T) (*RollingUpdateCluster, *openstack.MockCloud) { + vfs.Context.ResetMemfsContext(true) + + k8sClient := fake.NewSimpleClientset() + + mockcloud := testutils.SetupMockOpenstack() + + inCluster := testutils.BuildMinimalCluster("test.k8s.local") + + inCluster.Spec.CloudProvider = "openstack" + inCluster.Name = "test.k8s.local" + + inCluster.Spec.Topology.Masters = kopsapi.TopologyPrivate + inCluster.Spec.Topology.Nodes = kopsapi.TopologyPrivate + + err := cloudup.PerformAssignments(inCluster, mockcloud) + if err != nil { + t.Fatalf("Failed to perform assignments: %v", err) + } + + assetBuilder := assets.NewAssetBuilder(inCluster, "") + basePath, _ := vfs.Context.BuildVfsPath(inCluster.Spec.ConfigBase) + clientset := vfsclientset.NewVFSClientset(basePath) + cluster, err := cloudup.PopulateClusterSpec(clientset, inCluster, mockcloud, assetBuilder) + + if err != nil { + t.Fatalf("Failed to populate cluster spec: %v", err) + } + + sshPublicKey := []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDF2sghZsClUBXJB4mBMIw8rb0hJWjg1Vz4eUeXwYmTdi92Gf1zNc5xISSip9Y+PWX/jJokPB7tgPnMD/2JOAKhG1bi4ZqB15pYRmbbBekVpM4o4E0dx+czbqjiAm6wlccTrINK5LYenbucAAQt19eH+D0gJwzYUK9SYz1hWnlGS+qurt2bz7rrsG73lN8E2eiNvGtIXqv3GabW/Hea3acOBgCUJQWUDTRu0OmmwxzKbFN/UpNKeRaHlCqwZWjVAsmqA8TX8LIocq7Np7MmIBwt7EpEeZJxThcmC8DEJs9ClAjD+jlLIvMPXKC3JWCPgwCLGxHjy7ckSGFCSzbyPduh") + sshCredentialStore, err := clientset.SSHCredentialStore(cluster) + if err != nil { + t.Fatalf("Failed to get credential store: %v", err) + } + + sshCredentialStore.AddSSHPublicKey(fi.SecretNameSSHPrimary, sshPublicKey) + + c := &RollingUpdateCluster{ + Cloud: mockcloud, + MasterInterval: 1 * time.Millisecond, + NodeInterval: 1 * time.Millisecond, + BastionInterval: 1 * time.Millisecond, + Force: false, + K8sClient: k8sClient, + ClusterValidator: &successfulClusterValidator{}, + FailOnValidate: true, + ValidateTickDuration: 1 * time.Millisecond, + ValidateSuccessDuration: 5 * time.Millisecond, + ValidateCount: 2, + Ctx: context.Background(), + Cluster: cluster, + Clientset: clientset, + } + + return c, mockcloud +} + +func TestRollingUpdateDisabledSurgeOS(t *testing.T) { + origRegion := os.Getenv("OS_REGION_NAME") + os.Setenv("OS_REGION_NAME", "us-test1") + defer func() { + os.Setenv("OS_REGION_NAME", origRegion) + }() + + c, cloud := getTestSetupOS(t) + + groups, igList := getGroupsAllNeedUpdateOS(t, c) + err := c.RollingUpdate(groups, igList) + assert.NoError(t, err, "rolling update") + + assertGroupInstanceCountOS(t, cloud, "node-1", 3) + assertGroupInstanceCountOS(t, cloud, "node-2", 3) + assertGroupInstanceCountOS(t, cloud, "master-1", 2) + assertGroupInstanceCountOS(t, cloud, "bastion-1", 1) +} + +func makeGroupOS(t *testing.T, groups map[string]*cloudinstances.CloudInstanceGroup, igList *kopsapi.InstanceGroupList, + c *RollingUpdateCluster, subnet string, role kopsapi.InstanceGroupRole, count int, needUpdate int) { + cloud := c.Cloud.(*openstack.MockCloud) + igif := c.Clientset.InstanceGroupsFor(c.Cluster) + fakeClient := c.K8sClient.(*fake.Clientset) + + var newIg kopsapi.InstanceGroup + switch role { + case kopsapi.InstanceGroupRoleNode: + newIg = testutils.BuildMinimalNodeInstanceGroup("nodes-"+subnet, subnet) + case kopsapi.InstanceGroupRoleMaster: + newIg = testutils.BuildMinimalMasterInstanceGroup(subnet) + case kopsapi.InstanceGroupRoleBastion: + newIg = testutils.BuildMinimalBastionInstanceGroup("bastion-"+subnet, subnet) + } + + newIg.Spec.MachineType = "n1-standard-2" + newIg.Spec.Image = "Ubuntu-20.04" + + igList.Items = append(igList.Items, newIg) + + ig, err := igif.Create(c.Ctx, &newIg, v1meta.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create ig %v: %v", subnet, err) + } + + groups[subnet] = &cloudinstances.CloudInstanceGroup{ + HumanName: ig.ObjectMeta.Name, + InstanceGroup: ig, + } + for i := 0; i < count; i++ { + name := subnet + string(rune('a'+i)) + port, err := cloud.CreatePort(ports.CreateOpts{ + Name: name, + NetworkID: "test", + }) + if err != nil { + t.Fatalf("Failed to make port: %v", err) + } + server, err := cloud.CreateInstance(servers.CreateOpts{ + Name: name, + Networks: []servers.Network{ + { + Port: port.ID, + }, + }, + }) + if err != nil { + t.Fatalf("Failed to make group: %v", err) + } + id := server.ID + var node *v1.Node + if role != kopsapi.InstanceGroupRoleBastion { + node = &v1.Node{ + ObjectMeta: v1meta.ObjectMeta{Name: id + ".local"}, + } + _ = fakeClient.Tracker().Add(node) + } + member := cloudinstances.CloudInstance{ + ID: id, + Node: node, + CloudInstanceGroup: groups[subnet], + } + if i < needUpdate { + groups[subnet].NeedUpdate = append(groups[subnet].NeedUpdate, &member) + } else { + groups[subnet].Ready = append(groups[subnet].Ready, &member) + } + } +} + +func getGroupsAllNeedUpdateOS(t *testing.T, c *RollingUpdateCluster) (map[string]*cloudinstances.CloudInstanceGroup, *kopsapi.InstanceGroupList) { + groups := make(map[string]*cloudinstances.CloudInstanceGroup) + igList := &kopsapi.InstanceGroupList{} + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[0].Name, kopsapi.InstanceGroupRoleNode, 3, 3) + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[1].Name, kopsapi.InstanceGroupRoleNode, 3, 3) + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[0].Name, kopsapi.InstanceGroupRoleMaster, 1, 1) + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[1].Name, kopsapi.InstanceGroupRoleMaster, 1, 1) + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[2].Name, kopsapi.InstanceGroupRoleMaster, 1, 1) + makeGroupOS(t, groups, igList, c, c.Cluster.Spec.Subnets[0].Name, kopsapi.InstanceGroupRoleBastion, 1, 1) + return groups, igList +} + +func assertGroupInstanceCountOS(t *testing.T, cloud *openstack.MockCloud, groupName string, expected int) { + + groups, _ := cloud.ListServerGroups() + for _, g := range groups { + if g.Name == groupName { + assert.Lenf(t, g.Members, expected, "%s instances", groupName) + } + } +} diff --git a/pkg/model/openstackmodel/servergroup_test.go b/pkg/model/openstackmodel/servergroup_test.go index 7c1f7ed77f47d..072378eb885c7 100644 --- a/pkg/model/openstackmodel/servergroup_test.go +++ b/pkg/model/openstackmodel/servergroup_test.go @@ -986,7 +986,7 @@ func RunGoldenTest(t *testing.T, basedir string, testCase serverGroupModelBuilde defer h.Close() h.MockKopsVersion("1.18.0") - h.SetupMockOpenstack() + testutils.SetupMockOpenstack() clusterLifecycle := fi.LifecycleSync bootstrapScriptBuilder := &model.BootstrapScriptBuilder{ diff --git a/pkg/testutils/integrationtestharness.go b/pkg/testutils/integrationtestharness.go index f71515c06ff5e..312aa1308789d 100644 --- a/pkg/testutils/integrationtestharness.go +++ b/pkg/testutils/integrationtestharness.go @@ -253,7 +253,7 @@ func (h *IntegrationTestHarness) SetupMockGCE() { gce.InstallMockGCECloud("us-test1", "testproject") } -func (h *IntegrationTestHarness) SetupMockOpenstack() *openstack.MockCloud { +func SetupMockOpenstack() *openstack.MockCloud { c := openstack.InstallMockOpenstackCloud("us-test1") c.MockCinderClient = mockblockstorage.CreateClient() diff --git a/upup/pkg/fi/cloudup/openstack/instance.go b/upup/pkg/fi/cloudup/openstack/instance.go index ee9752ee60122..e9932312d5f77 100644 --- a/upup/pkg/fi/cloudup/openstack/instance.go +++ b/upup/pkg/fi/cloudup/openstack/instance.go @@ -117,7 +117,6 @@ func (c *openstackCloud) DeleteInstance(i *cloudinstances.CloudInstance) error { } func deleteInstance(c OpenstackCloud, i *cloudinstances.CloudInstance) error { - klog.Warning("This does not work without running kops update cluster --yes in another terminal") return deleteInstanceWithID(c, i.ID) }