Skip to content

Commit 8dd0d22

Browse files
committed
novncproxy cert and routes cleanup
Add tests for novncproxy certs and routes cleanup Closes: OSPRH-10549
1 parent 49734b3 commit 8dd0d22

4 files changed

Lines changed: 394 additions & 49 deletions

File tree

pkg/openstack/common.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,21 @@ func DeleteCertificate(
815815
helper.GetLogger().Info(fmt.Sprintf("Deleting cert %s", certName))
816816
return cert.Delete(ctx, helper)
817817
}
818+
819+
// // this checks if cert can be deleted on the basis of cellName
820+
// // Note: this is specific for certs/Route Name where cellName comes at index 2 - [0, 1, 2]
821+
// // ex: nova-novncproxy-cell1-public-svc
822+
// // but not: rabbitmq-cell1-svc
823+
// func ShouldCertRouteBeDeleted(
824+
// strObject string,
825+
// prefixSubstring string,
826+
// instance *corev1beta1.OpenStackControlPlane,
827+
// ) bool {
828+
// if strings.HasPrefix(strObject, prefixSubstring) {
829+
// cell := strings.Split(strObject, "-")[2]
830+
// if _, exists := (instance.Spec.Nova.Template.CellTemplates)[cell]; !exists {
831+
// return true
832+
// }
833+
// }
834+
// return false
835+
// }

pkg/openstack/nova.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,191 @@ package openstack
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
23+
"strings"
2224

2325
"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
2426
"github.com/openstack-k8s-operators/lib-common/modules/common/clusterdns"
2527
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
2628
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
29+
"github.com/openstack-k8s-operators/lib-common/modules/common/object"
2730
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
2831
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2932
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
3033

34+
"sigs.k8s.io/controller-runtime/pkg/client"
3135
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3236

3337
certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
3438
novav1 "github.com/openstack-k8s-operators/nova-operator/api/v1beta1"
3539
corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1"
40+
corev1 "k8s.io/api/core/v1"
3641
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
3742
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3843
"k8s.io/apimachinery/pkg/types"
3944
"k8s.io/utils/ptr"
4045
ctrl "sigs.k8s.io/controller-runtime"
4146
)
4247

48+
// func deleteUndefinedNoVNCProxyCertsByCellName(
49+
// ctx context.Context,
50+
// instance *corev1beta1.OpenStackControlPlane,
51+
// helper *helper.Helper,
52+
// ) (ctrl.Result, error) {
53+
54+
// log := GetLogger(ctx)
55+
// // Fetch the list of certificates
56+
// allCerts := &certmgrv1.CertificateList{}
57+
// listOpts := []client.ListOption{
58+
// client.InNamespace(instance.GetNamespace()),
59+
// }
60+
// err := helper.GetClient().List(ctx, allCerts, listOpts...)
61+
// if err != nil {
62+
// return ctrl.Result{}, fmt.Errorf("could not get certs %w", err)
63+
// }
64+
65+
// allRoutes := &routev1.RouteList{}
66+
// listOpts = []client.ListOption{
67+
// client.InNamespace(instance.GetNamespace()),
68+
// }
69+
// err = helper.GetClient().List(ctx, allRoutes, listOpts...)
70+
// if err != nil {
71+
// return ctrl.Result{}, fmt.Errorf("could not get routes %w", err)
72+
// }
73+
74+
// novncproxyPrefix := "nova-novncproxy-cell"
75+
76+
// var delErrs []error
77+
// for _, cert := range allCerts.Items {
78+
// shouldDelete := ShouldCertRouteBeDeleted(cert.Name, novncproxyPrefix, instance)
79+
// if shouldDelete {
80+
// if object.CheckOwnerRefExist(instance.GetUID(), cert.OwnerReferences) {
81+
// log.Info("Deleting novncproxy cert %s", "", cert.Name)
82+
// err = DeleteCertificate(ctx, helper, instance.Namespace, cert.Name)
83+
// if err != nil {
84+
// delErrs = append(delErrs, fmt.Errorf("novncproxy cert deletion for '%s' failed, because: %w", cert.Name, err))
85+
// }
86+
// }
87+
88+
// }
89+
// }
90+
91+
// for _, route := range allRoutes.Items {
92+
// shouldDelete := ShouldCertRouteBeDeleted(route.Name, novncproxyPrefix, instance)
93+
// if shouldDelete {
94+
// if object.CheckOwnerRefExist(instance.GetUID(), route.OwnerReferences) {
95+
// log.Info("Deleting novncproxy route for %s", "", route.Name)
96+
// _, err := EnsureDeleted(ctx, helper, &route)
97+
// if err != nil {
98+
// delErrs = append(delErrs, fmt.Errorf("novncproxy route deletion for '%s' failed, because: %w", route.Name, err))
99+
// }
100+
// }
101+
// }
102+
// }
103+
104+
// if len(delErrs) > 0 {
105+
// delErrs := errors.Join(delErrs...)
106+
// return ctrl.Result{}, delErrs
107+
// }
108+
109+
// return ctrl.Result{}, nil
110+
// }
111+
112+
func deleteCertsAndRoutes(
113+
ctx context.Context,
114+
instance *corev1beta1.OpenStackControlPlane,
115+
helper *helper.Helper,
116+
) (ctrl.Result, error) {
117+
118+
log := GetLogger(ctx)
119+
120+
novaNovncProxy := "nova-novncproxy"
121+
novncProxyLabelSelector := map[string]string{
122+
"osctlplane-service": novaNovncProxy,
123+
}
124+
125+
routes, err := GetRoutesListWithLabel(
126+
ctx,
127+
helper,
128+
instance.Namespace,
129+
novncProxyLabelSelector,
130+
)
131+
132+
if err != nil {
133+
return ctrl.Result{}, fmt.Errorf("could not get routes %w", err)
134+
}
135+
136+
certs := &certmgrv1.CertificateList{}
137+
listOpts := []client.ListOption{
138+
client.InNamespace(instance.Namespace),
139+
}
140+
141+
err = helper.GetClient().List(ctx, certs, listOpts...)
142+
if err != nil {
143+
return ctrl.Result{}, fmt.Errorf("could not get all certs %w", err)
144+
}
145+
146+
var delErrs []error
147+
for _, route := range routes.Items {
148+
log.Info("", "xxx- route", route.Name, "---", route.Spec.To.Name)
149+
150+
// route.Spec.To.Name nova-novncproxy-cell1-public"
151+
svc := &corev1.Service{}
152+
err := helper.GetClient().Get(ctx, types.NamespacedName{
153+
Name: route.Spec.To.Name,
154+
Namespace: instance.Namespace,
155+
}, svc)
156+
157+
if err != nil {
158+
159+
if !k8s_errors.IsNotFound(err) {
160+
// its different error
161+
// do not delete yet.
162+
log.Error(err, "xxx- Error")
163+
continue
164+
}
165+
166+
log.Info("", "xxx- can delete", route.Name)
167+
168+
if !object.CheckOwnerRefExist(instance.GetUID(), route.OwnerReferences) {
169+
continue
170+
}
171+
172+
// service not found for route, clean certs and route
173+
for _, cert := range certs.Items {
174+
// as we have 2 with cell-name and -vencrypt, so this is generic
175+
if strings.Contains(cert.Name, novaNovncProxy) {
176+
// log.Info("", "t- cert", "-------", "Name", cert.Name)
177+
if object.CheckOwnerRefExist(instance.GetUID(), cert.OwnerReferences) {
178+
log.Info("xxx- ", "Deleting novncproxy cert %s", "", cert.Name)
179+
err = DeleteCertificate(ctx, helper, instance.Namespace, cert.Name)
180+
if err != nil {
181+
delErrs = append(delErrs, fmt.Errorf("novncproxy cert deletion for '%s' failed, because: %w", cert.Name, err))
182+
}
183+
}
184+
}
185+
}
186+
187+
log.Info("xxx-", "Deleting novncproxy route for %s", "", route.Name)
188+
_, err := EnsureDeleted(ctx, helper, &route)
189+
if err != nil {
190+
delErrs = append(delErrs, fmt.Errorf("novncproxy route deletion for '%s' failed, because: %w", route.Name, err))
191+
}
192+
193+
} else {
194+
log.Info("", "xxx- can't delete", route.Name)
195+
}
196+
}
197+
198+
if len(delErrs) > 0 {
199+
delErrs := errors.Join(delErrs...)
200+
return ctrl.Result{}, delErrs
201+
}
202+
203+
return ctrl.Result{}, nil
204+
}
205+
43206
// ReconcileNova -
44207
func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *helper.Helper) (ctrl.Result, error) {
45208
nova := &novav1.Nova{
@@ -209,6 +372,7 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
209372
}
210373

211374
// cell Metadata and NoVNCProxy
375+
Log.Info("", "", "XXX ReconcileNova iterating cells")
212376
for cellName, cellTemplate := range instance.Spec.Nova.Template.CellTemplates {
213377
// create certificate for Metadata agend if internal TLS and Metadata per cell is enabled
214378
if instance.Spec.TLS.PodLevel.Enabled &&
@@ -232,6 +396,7 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
232396

233397
// NoVNCProxy check for/creating route if service is enabled
234398
if noVNCProxyEnabled(cellTemplate.NoVNCProxyServiceTemplate) {
399+
Log.Info("", "XXX ReconcileNova VNC enabled cell name", cellName)
235400
if cellTemplate.NoVNCProxyServiceTemplate.Override.Service == nil {
236401
cellTemplate.NoVNCProxyServiceTemplate.Override.Service = &service.RoutedOverrideSpec{}
237402
}
@@ -247,6 +412,7 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
247412
}
248413

249414
// make sure to get to EndpointConfig when all service got created
415+
Log.Info("", "XXX vnc services", svcs.Items)
250416
if len(svcs.Items) == 1 {
251417
endpointDetails, ctrlResult, err := EnsureEndpointConfig(
252418
ctx,
@@ -394,6 +560,18 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
394560
corev1beta1.OpenStackControlPlaneNovaReadyRunningMessage))
395561
}
396562

563+
// _, errs := deleteUndefinedNoVNCProxyCertsByCellName(ctx, instance, helper)
564+
_, errs := deleteCertsAndRoutes(ctx, instance, helper)
565+
if errs != nil {
566+
instance.Status.Conditions.Set(condition.FalseCondition(
567+
corev1beta1.OpenStackControlPlaneNovaReadyCondition,
568+
condition.ErrorReason,
569+
condition.SeverityWarning,
570+
corev1beta1.OpenStackControlPlaneNovaReadyErrorMessage,
571+
errs))
572+
return ctrl.Result{}, errs
573+
}
574+
397575
return ctrl.Result{}, nil
398576
}
399577

tests/functional/ctlplane/base_test.go

Lines changed: 56 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -44,47 +44,50 @@ import (
4444
)
4545

4646
type Names struct {
47-
Namespace string
48-
OpenStackControlplaneName types.NamespacedName
49-
OpenStackVersionName types.NamespacedName
50-
KeystoneAPIName types.NamespacedName
51-
MemcachedName types.NamespacedName
52-
MemcachedCertName types.NamespacedName
53-
CinderName types.NamespacedName
54-
ManilaName types.NamespacedName
55-
GlanceName types.NamespacedName
56-
NeutronName types.NamespacedName
57-
HorizonName types.NamespacedName
58-
HeatName types.NamespacedName
59-
TelemetryName types.NamespacedName
60-
DBName types.NamespacedName
61-
DBCertName types.NamespacedName
62-
DBCell1Name types.NamespacedName
63-
DBCell1CertName types.NamespacedName
64-
RabbitMQName types.NamespacedName
65-
RabbitMQCertName types.NamespacedName
66-
RabbitMQCell1Name types.NamespacedName
67-
RabbitMQCell1CertName types.NamespacedName
68-
ServiceAccountName types.NamespacedName
69-
RoleName types.NamespacedName
70-
RoleBindingName types.NamespacedName
71-
RootCAPublicName types.NamespacedName
72-
RootCAInternalName types.NamespacedName
73-
RootCAOvnName types.NamespacedName
74-
RootCALibvirtName types.NamespacedName
75-
SelfSignedIssuerName types.NamespacedName
76-
CustomIssuerName types.NamespacedName
77-
CustomServiceCertSecretName types.NamespacedName
78-
CABundleName types.NamespacedName
79-
OpenStackClientName types.NamespacedName
80-
OVNNorthdName types.NamespacedName
81-
OVNNorthdCertName types.NamespacedName
82-
OVNControllerName types.NamespacedName
83-
OVNControllerCertName types.NamespacedName
84-
OVNDbServerNBName types.NamespacedName
85-
OVNDbServerSBName types.NamespacedName
86-
NeutronOVNCertName types.NamespacedName
87-
OpenStackTopology []types.NamespacedName
47+
Namespace string
48+
OpenStackControlplaneName types.NamespacedName
49+
OpenStackVersionName types.NamespacedName
50+
KeystoneAPIName types.NamespacedName
51+
MemcachedName types.NamespacedName
52+
MemcachedCertName types.NamespacedName
53+
CinderName types.NamespacedName
54+
ManilaName types.NamespacedName
55+
GlanceName types.NamespacedName
56+
NeutronName types.NamespacedName
57+
HorizonName types.NamespacedName
58+
HeatName types.NamespacedName
59+
TelemetryName types.NamespacedName
60+
DBName types.NamespacedName
61+
DBCertName types.NamespacedName
62+
DBCell1Name types.NamespacedName
63+
DBCell1CertName types.NamespacedName
64+
RabbitMQName types.NamespacedName
65+
RabbitMQCertName types.NamespacedName
66+
RabbitMQCell1Name types.NamespacedName
67+
RabbitMQCell1CertName types.NamespacedName
68+
NoVNCProxyCell1CertPublicRouteName types.NamespacedName
69+
NoVNCProxyCell1CertPublicSvcName types.NamespacedName
70+
NoVNCProxyCell1CertVencryptName types.NamespacedName
71+
ServiceAccountName types.NamespacedName
72+
RoleName types.NamespacedName
73+
RoleBindingName types.NamespacedName
74+
RootCAPublicName types.NamespacedName
75+
RootCAInternalName types.NamespacedName
76+
RootCAOvnName types.NamespacedName
77+
RootCALibvirtName types.NamespacedName
78+
SelfSignedIssuerName types.NamespacedName
79+
CustomIssuerName types.NamespacedName
80+
CustomServiceCertSecretName types.NamespacedName
81+
CABundleName types.NamespacedName
82+
OpenStackClientName types.NamespacedName
83+
OVNNorthdName types.NamespacedName
84+
OVNNorthdCertName types.NamespacedName
85+
OVNControllerName types.NamespacedName
86+
OVNControllerCertName types.NamespacedName
87+
OVNDbServerNBName types.NamespacedName
88+
OVNDbServerSBName types.NamespacedName
89+
NeutronOVNCertName types.NamespacedName
90+
OpenStackTopology []types.NamespacedName
8891
}
8992

9093
func CreateNames(openstackControlplaneName types.NamespacedName) Names {
@@ -200,6 +203,18 @@ func CreateNames(openstackControlplaneName types.NamespacedName) Names {
200203
Namespace: openstackControlplaneName.Namespace,
201204
Name: "cert-rabbitmq-cell1-svc",
202205
},
206+
NoVNCProxyCell1CertPublicRouteName: types.NamespacedName{
207+
Name: "cert-nova-novncproxy-cell1-public-route",
208+
Namespace: openstackControlplaneName.Namespace,
209+
},
210+
NoVNCProxyCell1CertPublicSvcName: types.NamespacedName{
211+
Name: "cert-nova-novncproxy-cell1-public-svc",
212+
Namespace: openstackControlplaneName.Namespace,
213+
},
214+
NoVNCProxyCell1CertVencryptName: types.NamespacedName{
215+
Name: "cert-nova-novncproxy-cell1-vencrypt",
216+
Namespace: openstackControlplaneName.Namespace,
217+
},
203218
OpenStackClientName: types.NamespacedName{
204219
Namespace: openstackControlplaneName.Namespace,
205220
Name: "openstackclient",

0 commit comments

Comments
 (0)