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

Clean conductors from db during rescaling #620

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/v1beta1/novaconductor_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,13 @@ func NewNovaConductorSpec(
func (n NovaConductor) GetSecret() string {
return n.Spec.Secret
}

// GetKeystoneAuthURL returns the KeystoneAuthURL from the Spec
func (n NovaConductor) GetKeystoneAuthURL() string {
return n.Spec.KeystoneAuthURL
}

// GetServiceUser returns the Service user from the Spec
func (n NovaConductor) GetKeystoneUser() string {
return n.Spec.ServiceUser
}
10 changes: 10 additions & 0 deletions api/v1beta1/novascheduler_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,13 @@ func (s NovaSchedulerStatus) GetConditions() condition.Conditions {
func (n NovaScheduler) GetSecret() string {
return n.Spec.Secret
}

// GetKeystoneAuthURL returns the KeystoneAuthURL from the Spec
func (n NovaScheduler) GetKeystoneAuthURL() string {
return n.Spec.KeystoneAuthURL
}

// GetServiceUser returns the Service user from the Spec
func (n NovaScheduler) GetKeystoneUser() string {
return n.Spec.ServiceUser
}
54 changes: 43 additions & 11 deletions controllers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/openstack-k8s-operators/nova-operator/pkg/nova"

gophercloud "github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/services"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
Expand Down Expand Up @@ -93,15 +94,39 @@ const (
// TransportURLSelector is the name of key in the internal cell
// Secret for the cell message bus transport URL
TransportURLSelector = "transport_url"

// defaultRequestTimeout is the default timeout duration for requests
defaultRequestTimeout = 10 * time.Second
)

type conditionsGetter interface {
GetConditions() condition.Conditions
}

func cleanNovaServiceFromNovaDb(
ctx context.Context,
computeClient *gophercloud.ServiceClient,
serviceName string,
) error {
opts := services.ListOpts{
Binary: serviceName,
}

allPages, err := services.List(computeClient, opts).AllPages()
if err != nil {
return err
}

allServices, err := services.ExtractServices(allPages)
if err != nil {
return err
}
for _, service := range allServices {
if service.State == "down" {
services.Delete(computeClient, service.ID)
}
}

return err
}

func allSubConditionIsTrue(conditionsGetter conditionsGetter) bool {
// It assumes that all of our conditions report success via the True status
for _, c := range conditionsGetter.GetConditions() {
Expand Down Expand Up @@ -532,25 +557,32 @@ func (r *ReconcilerBase) ensureMetadataDeleted(
return nil
}

func getNovaClient(ctx context.Context,
h *helper.Helper, authURL string, adminUser string, authPassword string, timeout time.Duration, l logr.Logger) (*gophercloud.ServiceClient, ctrl.Result, error) {
type keystoneAuth interface {
GetKeystoneAuthURL() string
GetKeystoneUser() string
}

func getNovaClient(
ctx context.Context, h *helper.Helper,
keystoneAuth keystoneAuth, password string,
l logr.Logger,
) (*gophercloud.ServiceClient, error) {

cfg := openstack.AuthOpts{
AuthURL: authURL,
Username: adminUser,
Password: authPassword,
AuthURL: keystoneAuth.GetKeystoneAuthURL(),
Username: keystoneAuth.GetKeystoneUser(),
Password: password,
DomainName: "Default", // fixme",
Region: "regionOne", // fixme",
TenantName: "service", // fixme",
}
// create the compute client using previous providerClient
endpointOpts := gophercloud.EndpointOpts{
Region: cfg.Region,
Availability: gophercloud.AvailabilityInternal,
}
computeClient, err := openstack.GetNovaOpenStackClient(l, cfg, endpointOpts)
if err != nil {
return nil, ctrl.Result{}, err
return nil, err
}
return computeClient.GetOSClient(), ctrl.Result{}, nil
return computeClient.GetOSClient(), nil
}
23 changes: 23 additions & 0 deletions controllers/novaconductor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ func (r *NovaConductorReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return result, err
}

// 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 {
Log.Error(err, "Failed cleaning services from nova db")
}

Log.Info("Successfully reconciled")
return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -446,6 +452,23 @@ func (r *NovaConductorReconciler) ensureDeployment(
return ctrl.Result{}, nil
}

func (r *NovaConductorReconciler) cleanServiceFromNovaDb(
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be generalized now as it only differs from the scheduler one by Binary: "nova-conductor" so that can be passed as a parameter and also the keystoneAuthURL can be taken as a parameter.

Copy link
Contributor Author

@mrkisaolamb mrkisaolamb Dec 8, 2023

Choose a reason for hiding this comment

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

done a68cc8c

ctx context.Context,
h *helper.Helper,
instance *novav1.NovaConductor,
secret corev1.Secret,
l logr.Logger,
) error {

authPassword := string(secret.Data[ServicePasswordSelector])
computeClient, err := getNovaClient(ctx, h, instance, authPassword, l)
if err != nil {
return err
}

return cleanNovaServiceFromNovaDb(ctx, computeClient, "nova-conductor")
}

// SetupWithManager sets up the controller with the Manager.
func (r *NovaConductorReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
23 changes: 2 additions & 21 deletions controllers/novascheduler_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/go-logr/logr"
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/services"
common "github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
Expand Down Expand Up @@ -422,28 +421,10 @@ func (r *NovaSchedulerReconciler) cleanServiceFromNovaDb(
l logr.Logger,
) error {
authPassword := string(secret.Data[ServicePasswordSelector])
computeClient, _, err := getNovaClient(ctx, h, instance.Spec.KeystoneAuthURL, instance.Spec.ServiceUser, authPassword, defaultRequestTimeout, l)
computeClient, err := getNovaClient(ctx, h, instance, authPassword, l)
if err != nil {
return err
}
opts := services.ListOpts{
Binary: "nova-scheduler",
}

allPages, err := services.List(computeClient, opts).AllPages()
if err != nil {
return err
}

allServices, err := services.ExtractServices(allPages)
if err != nil {
return err
}
for _, service := range allServices {
if service.State == "down" {
services.Delete(computeClient, service.ID)
}
}

return err
return cleanNovaServiceFromNovaDb(ctx, computeClient, "nova-scheduler")
}
11 changes: 11 additions & 0 deletions test/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,17 @@ func (f *NovaAPIFixture) ServicesList(w http.ResponseWriter, r *http.Request) {
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
},
{
"id": 4,
"binary": "nova-conductor",
"disabled_reason": "test4",
"host": "host2",
"state": "down",
"status": "disabled",
"updated_at": "2012-09-18T08:03:38.000000",
"forced_down": false,
"zone": "nova"
}
]
}
Expand Down
48 changes: 48 additions & 0 deletions test/functional/novaconductor_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ package functional_test

import (
"encoding/json"
"fmt"
"net/http"

networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
keystone_helper "github.com/openstack-k8s-operators/keystone-operator/api/test/helpers"
. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
api "github.com/openstack-k8s-operators/lib-common/modules/test/apis"
novav1 "github.com/openstack-k8s-operators/nova-operator/api/v1beta1"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -637,3 +641,47 @@ var _ = Describe("NovaConductor controller", func() {
})
})
})

var _ = Describe("NovaConductor controller cleaning", func() {
var novaAPIServer *NovaAPIFixture
BeforeEach(func() {
novaAPIServer = NewNovaAPIFixtureWithServer(logger)
novaAPIServer.Setup()
f := keystone_helper.NewKeystoneAPIFixtureWithServer(logger)
text := ResponseHandleToken(f.Endpoint(), novaAPIServer.Endpoint())
f.Setup(
api.Handler{Pattern: "/", Func: f.HandleVersion},
api.Handler{Pattern: "/v3/users", Func: f.HandleUsers},
api.Handler{Pattern: "/v3/domains", Func: f.HandleDomains},
api.Handler{Pattern: "/v3/auth/tokens", Func: func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "POST":
w.Header().Add("Content-Type", "application/json")
w.WriteHeader(202)
fmt.Fprint(w, text)
}
}})
DeferCleanup(f.Cleanup)

spec := GetDefaultNovaConductorSpec(cell0)
spec["keystoneAuthURL"] = f.Endpoint()
DeferCleanup(th.DeleteInstance, CreateNovaConductor(cell0.ConductorName, spec))
DeferCleanup(
k8sClient.Delete, ctx, CreateCellInternalSecret(cell0))
th.SimulateJobSuccess(cell0.DBSyncJobName)
th.SimulateStatefulSetReplicaReady(cell0.ConductorStatefulSetName)
})
When("NovaConductor down service is removed from api", func() {
It("during reconciling", func() {
th.SimulateStatefulSetReplicaReady(cell0.ConductorStatefulSetName)
th.ExpectCondition(
cell0.ConductorName,
ConditionGetterFunc(NovaConductorConditionGetter),
condition.DeploymentReadyCondition,
corev1.ConditionTrue,
)
Expect(novaAPIServer.FindRequest("GET", "/compute/os-services/", "binary=nova-conductor")).To(BeTrue())
Expect(novaAPIServer.FindRequest("DELETE", "/compute/os-services/3", "")).To(BeTrue())
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that fixture records the query param please assert that the code queried a list of conductor services.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done a68cc8c

Copy link
Contributor

Choose a reason for hiding this comment

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

cool. As a follow up add the same to the schedule test

})
})
})
Loading