Skip to content

Commit

Permalink
Adds a check for scaled down service
Browse files Browse the repository at this point in the history
Clean up scaled down services based on hostname

- Update service objects in base_test
- Delete service host from nova DB by hostname on replica count update
  • Loading branch information
stack authored and auniyal61 committed Jun 3, 2024
1 parent 31755a1 commit 71dedbe
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 13 deletions.
17 changes: 16 additions & 1 deletion controllers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"fmt"
"net/url"
"sort"
"strconv"
"strings"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -115,6 +117,7 @@ func cleanNovaServiceFromNovaDb(
computeClient *gophercloud.ServiceClient,
serviceName string,
l logr.Logger,
replicaCount int32,
) error {
opts := services.ListOpts{
Binary: serviceName,
Expand All @@ -131,7 +134,19 @@ func cleanNovaServiceFromNovaDb(
}

for _, service := range allServices {
if service.State == "down" {
// extract 0 (suffix) from hostname nova-scheduler-0
hostSplits := strings.Split(service.Host, "-")
hostIndexStr := hostSplits[len(hostSplits)-1]

hostIndex, err := strconv.Atoi(hostIndexStr)
if err != nil {
return err
}

// name index start from 0
// which means if replicaCount is 1, only nova-scheduler-0 is valid case
// so delete >= 1
if service.State == "down" || hostIndex >= int(replicaCount) {
rsp := services.Delete(computeClient, service.ID)
if rsp.Err != nil {
l.Error(rsp.Err, "Failed to delete service", "service", service, "response", rsp)
Expand Down
6 changes: 5 additions & 1 deletion controllers/novaconductor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return ctrl.Result{}, err
}

if instance.Spec.Replicas != &instance.Status.ReadyCount {
return ctrl.Result{}, nil
}
// clean up nova services from nova db should be always a last step in reconcile
err = r.cleanServiceFromNovaDb(ctx, h, instance, secret, Log)
if err != nil {
Expand Down Expand Up @@ -609,13 +612,14 @@ func (r *NovaConductorReconciler) cleanServiceFromNovaDb(
secret corev1.Secret,
l logr.Logger,
) error {
replicaCount := instance.Spec.Replicas
authPassword := string(secret.Data[ServicePasswordSelector])
computeClient, err := getNovaClient(ctx, h, instance, authPassword, l)
if err != nil {
return err
}

return cleanNovaServiceFromNovaDb(computeClient, "nova-conductor", l)
return cleanNovaServiceFromNovaDb(computeClient, "nova-conductor", l, *replicaCount)
}

func (r *NovaConductorReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
Expand Down
7 changes: 6 additions & 1 deletion controllers/novascheduler_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ func (r *NovaSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return result, err
}

if instance.Spec.Replicas != &instance.Status.ReadyCount {
return ctrl.Result{}, nil
}

// clean up nova services from nova db should be always a last step in reconcile
// to make sure that
err = r.cleanServiceFromNovaDb(ctx, h, instance, secret, Log)
Expand Down Expand Up @@ -663,11 +667,12 @@ func (r *NovaSchedulerReconciler) cleanServiceFromNovaDb(
secret corev1.Secret,
l logr.Logger,
) error {
replicaCount := instance.Spec.Replicas
authPassword := string(secret.Data[ServicePasswordSelector])
computeClient, err := getNovaClient(ctx, h, instance, authPassword, l)
if err != nil {
return err
}

return cleanNovaServiceFromNovaDb(computeClient, "nova-scheduler", l)
return cleanNovaServiceFromNovaDb(computeClient, "nova-scheduler", l, *replicaCount)
}
37 changes: 30 additions & 7 deletions test/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (f *NovaAPIFixture) ServicesList(w http.ResponseWriter, r *http.Request) {
"id": 1,
"binary": "nova-scheduler",
"disabled_reason": "test1",
"host": "host1",
"host": "nova-scheduler-0",
"state": "up",
"status": "disabled",
"updated_at": "2012-10-29T13:42:02.000000",
Expand All @@ -144,7 +144,7 @@ func (f *NovaAPIFixture) ServicesList(w http.ResponseWriter, r *http.Request) {
"id": 2,
"binary": "nova-compute",
"disabled_reason": "test2",
"host": "host1",
"host": "nova-compute-0",
"state": "up",
"status": "disabled",
"updated_at": "2012-10-29T13:42:05.000000",
Expand All @@ -155,7 +155,7 @@ func (f *NovaAPIFixture) ServicesList(w http.ResponseWriter, r *http.Request) {
"id": 3,
"binary": "nova-scheduler",
"disabled_reason": null,
"host": "host2",
"host": "nova-scheduler-1",
"state": "down",
"status": "enabled",
"updated_at": "2012-09-19T06:55:34.000000",
Expand All @@ -166,24 +166,47 @@ func (f *NovaAPIFixture) ServicesList(w http.ResponseWriter, r *http.Request) {
"id": 4,
"binary": "nova-compute",
"disabled_reason": "test4",
"host": "host2",
"host": "nova-compute-1",
"state": "down",
"status": "disabled",
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
},
{
"id": 4,
"id": 5,
"binary": "nova-conductor",
"disabled_reason": "test4",
"host": "host2",
"disabled_reason": "test5",
"host": "nova-cell0-conductor-0",
"state": "down",
"status": "disabled",
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
},
{
"id": 6,
"binary": "nova-conductor",
"disabled_reason": "test6",
"host": "nova-cell1-conductor-0",
"state": "up",
"status": "disabled",
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
},
{
"id": 7,
"binary": "nova-conductor",
"disabled_reason": "test7",
"host": "nova-cell1-conductor-1",
"state": "down",
"status": "disabled",
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
}
]
}
`)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/nova_scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ var _ = Describe("NovaScheduler controller cleaning", func() {
condition.ReadyCondition,
corev1.ConditionTrue,
)
Expect(novaAPIFixture.HasRequest("DELETE", "/compute/os-services/3", "")).To(BeTrue())
Expect(novaAPIFixture.HasRequest("DELETE", "/compute/os-services/3", "binary=nova-scheduler")).To(BeTrue())
req := novaAPIFixture.FindRequest("DELETE", "/compute/os-services/3", "")
Expect(req.Header.Get("X-OpenStack-Nova-API-Version")).To(Equal("2.95"))

Expand Down
4 changes: 2 additions & 2 deletions test/functional/novaconductor_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,8 +798,8 @@ var _ = Describe("NovaConductor controller cleaning", func() {
corev1.ConditionTrue,
)
Expect(novaAPIServer.HasRequest("GET", "/compute/os-services/", "binary=nova-conductor")).To(BeTrue())
Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/3", "")).To(BeTrue())
req := novaAPIServer.FindRequest("DELETE", "/compute/os-services/3", "")
Expect(novaAPIServer.HasRequest("DELETE", "/compute/os-services/5", "")).To(BeTrue())
req := novaAPIServer.FindRequest("DELETE", "/compute/os-services/5", "")
Expect(req.Header.Get("X-OpenStack-Nova-API-Version")).To(Equal("2.95"))
})
})
Expand Down

0 comments on commit 71dedbe

Please sign in to comment.