Skip to content

Commit

Permalink
save
Browse files Browse the repository at this point in the history
  • Loading branch information
kon-angelo committed Jun 10, 2024
1 parent 3275f3d commit a111f7b
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ mountOptions:
parameters:
type: default
shareNetworkID: {{ $.Values.openstack.shareNetworkID }}
{{- if $.Values.openstack.shareClient}}
nfs-shareClient: {{ required "openstack.shareClient needs to be set" $.Values.openstack.shareClient }}
{{- end }}
csi.storage.k8s.io/provisioner-secret-name: manila-csi-plugin
csi.storage.k8s.io/provisioner-secret-namespace: {{ $.Release.Namespace }}
csi.storage.k8s.io/node-stage-secret-name: manila-csi-plugin
Expand Down Expand Up @@ -43,7 +45,9 @@ parameters:
type: default
autoTopology: "true"
shareNetworkID: {{ $.Values.openstack.shareNetworkID }}
{{- if $.Values.openstack.shareClient}}
nfs-shareClient: {{ required "openstack.shareClient needs to be set" $.Values.openstack.shareClient }}
{{- end }}
csi.storage.k8s.io/provisioner-secret-name: manila-csi-plugin
csi.storage.k8s.io/provisioner-secret-namespace: {{ $.Release.Namespace }}
csi.storage.k8s.io/node-stage-secret-name: manila-csi-plugin
Expand Down Expand Up @@ -72,7 +76,9 @@ parameters:
type: default
availability: {{ required "openstack.availabilityZones needs to be set" . }}
shareNetworkID: {{ $.Values.openstack.shareNetworkID }}
{{- if $.Values.openstack.shareClient}}
nfs-shareClient: {{ required "openstack.shareClient needs to be set" $.Values.openstack.shareClient }}
{{- end }}
csi.storage.k8s.io/provisioner-secret-name: manila-csi-plugin
csi.storage.k8s.io/provisioner-secret-namespace: {{ $.Release.Namespace }}
csi.storage.k8s.io/node-stage-secret-name: manila-csi-plugin
Expand Down Expand Up @@ -107,7 +113,9 @@ parameters:
type: default
availability: {{ required "openstack.availabilityZones needs to be set" . }}
shareNetworkID: {{ $.Values.openstack.shareNetworkID }}
{{- if $.Values.openstack.shareClient}}
nfs-shareClient: {{ required "openstack.shareClient needs to be set" $.Values.openstack.shareClient }}
{{- end }}
csi.storage.k8s.io/provisioner-secret-name: manila-csi-plugin
csi.storage.k8s.io/provisioner-secret-namespace: {{ $.Release.Namespace }}
csi.storage.k8s.io/node-stage-secret-name: manila-csi-plugin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ openstack:
- zone1
- zone2
shareNetworkID: shareNetworkIDValue
shareClient: 10.180.0.0/16
# shareClient: 10.180.0.0/16
shareProtocol: NFS
authURL: authURLValue
region: regionValue
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/openstack/validation/infrastructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func ValidateInfrastructureConfig(infra *api.InfrastructureConfig, nodesCIDR *st
}

networksPath := fldPath.Child("networks")
if len(infra.Networks.Worker) == 0 && len(infra.Networks.Workers) == 0 {
if len(infra.Networks.Worker) == 0 && len(infra.Networks.Workers) == 0 && infra.Networks.SubnetID == nil {
allErrs = append(allErrs, field.Required(networksPath.Child("workers"), "must specify the network range for the worker network"))
}

Expand Down
6 changes: 0 additions & 6 deletions pkg/controller/controlplane/valuesprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
"github.com/gardener/gardener-extension-provider-openstack/charts"
api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
"github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/helper"
"github.com/gardener/gardener-extension-provider-openstack/pkg/internal/infrastructure"
"github.com/gardener/gardener-extension-provider-openstack/pkg/openstack"
"github.com/gardener/gardener-extension-provider-openstack/pkg/utils"
)
Expand Down Expand Up @@ -952,10 +951,6 @@ func (vp *valuesProvider) addCSIManilaValues(
"clusterID": cp.Namespace,
}

infraConfig, err := helper.InfrastructureConfigFromRawExtension(cluster.Shoot.Spec.Provider.InfrastructureConfig)
if err != nil {
return fmt.Errorf("could not decode infrastructure config of controlplane '%s': %w", kutil.ObjectName(cp), err)
}
infraStatus, err := vp.getInfrastructureStatus(cp)
if err != nil {
return err
Expand Down Expand Up @@ -984,7 +979,6 @@ func (vp *valuesProvider) addCSIManilaValues(
values["openstack"] = map[string]interface{}{
"availabilityZones": vp.getAllWorkerPoolsZones(cluster),
"shareNetworkID": shareNetworkID,
"shareClient": infrastructure.WorkersCIDR(infraConfig),
"authURL": authURL,
"region": cp.Spec.Region,
"domainName": domainName,
Expand Down
20 changes: 10 additions & 10 deletions pkg/controller/controlplane/valuesprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -663,11 +663,11 @@ var _ = Describe("ValuesProvider", func() {
"authURL": authURL,
"region": "europe",
"applicationCredentialSecret": "",
"shareClient": "10.200.0.0/19",
"shareNetworkID": "1111-2222-3333-4444",
"domainName": "domain-name",
"tlsInsecure": "",
"caCert": "",
// "shareClient": "10.200.0.0/19",
"shareNetworkID": "1111-2222-3333-4444",
"domainName": "domain-name",
"tlsInsecure": "",
"caCert": "",
},
}),
}))
Expand Down Expand Up @@ -792,11 +792,11 @@ var _ = Describe("ValuesProvider", func() {
"authURL": authURL,
"region": "europe",
"applicationCredentialSecret": "",
"shareClient": "10.200.0.0/19",
"shareNetworkID": "1111-2222-3333-4444",
"domainName": "domain-name",
"tlsInsecure": "",
"caCert": "",
// "shareClient": "10.200.0.0/19",
"shareNetworkID": "1111-2222-3333-4444",
"domainName": "domain-name",
"tlsInsecure": "",
"caCert": "",
},
"vpaEnabled": true,
}),
Expand Down
11 changes: 8 additions & 3 deletions pkg/controller/infrastructure/infraflow/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (fctx *FlowContext) buildDeleteGraph() *flow.Graph {
needToDeleteNetwork := fctx.config.Networks.ID == nil
needToDeleteRouter := fctx.config.Networks.Router == nil
// skip deletion of the subnet if we need to delete the network (it will be deleted anyway), or if the subnet is user provided.
needToDeleteSubnet := !needToDeleteNetwork && fctx.config.Networks.SubnetID == nil
needToDeleteSubnet := fctx.config.Networks.SubnetID == nil

_ = fctx.AddTask(g, "delete ssh key pair",
fctx.deleteSSHKeyPair,
Expand All @@ -57,8 +57,13 @@ func (fctx *FlowContext) buildDeleteGraph() *flow.Graph {
if routerID == nil {
return nil
}
return infrastructure.CleanupKubernetesRoutes(ctx, fctx.networking, *routerID, infrastructure.WorkersCIDR(fctx.config))
subnetID := fctx.state.Get(IdentifierSubnet)
if subnetID == nil {
return nil
}
return infrastructure.CleanupKubernetesRoutes(ctx, fctx.networking, *routerID, *subnetID)
},
shared.DoIf(needToDeleteRouter || needToDeleteSubnet),
shared.Timeout(defaultTimeout),
)
k8sLoadBalancers := fctx.AddTask(g, "delete kubernetes loadbalancers",
Expand All @@ -82,7 +87,7 @@ func (fctx *FlowContext) buildDeleteGraph() *flow.Graph {
// subnet deletion only needed if network is given by spec
_ = fctx.AddTask(g, "delete subnet",
fctx.deleteSubnet,
shared.DoIf(needToDeleteSubnet), shared.Timeout(defaultTimeout), shared.Dependencies(deleteRouterInterface, k8sLoadBalancers))
shared.DoIf(!needToDeleteNetwork && needToDeleteSubnet), shared.Timeout(defaultTimeout), shared.Dependencies(deleteRouterInterface, k8sLoadBalancers))
_ = fctx.AddTask(g, "delete network",
fctx.deleteNetwork,
shared.DoIf(needToDeleteNetwork), shared.Timeout(defaultTimeout), shared.Dependencies(deleteRouterInterface))
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/infrastructure/infraflow/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (fctx *FlowContext) buildReconcileGraph() *flow.Graph {

_ = fctx.AddTask(g, "ensure router interface",
fctx.ensureRouterInterface,
shared.DoIf(fctx.config.Networks.SubnetID == nil || fctx.config.Networks.Router == nil),
shared.Timeout(defaultTimeout), shared.Dependencies(ensureRouter, ensureSubnet))

ensureSecGroup := fctx.AddTask(g, "ensure security group",
Expand Down Expand Up @@ -292,7 +293,7 @@ func (fctx *FlowContext) ensureNewSubnet(ctx context.Context) error {
desired := &subnets.Subnet{
Name: fctx.defaultSubnetName(),
NetworkID: networkID,
CIDR: fctx.workerCIDR(),
CIDR: infrainternal.WorkersCIDR(fctx.config),
IPVersion: 4,
DNSNameservers: fctx.cloudProfileConfig.DNSServers,
}
Expand Down
12 changes: 2 additions & 10 deletions pkg/controller/infrastructure/infraflow/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package infraflow

import (
"fmt"
"reflect"
)

// ErrorMultipleMatches is returned when the findExisting finds multiple resources matching a name.
Expand Down Expand Up @@ -36,7 +37,7 @@ func findExisting[T any](id *string, name string,

// TODO: check if this makes sense
if len(found) > 1 {
return nil, ErrorMultipleMatches
return nil, fmt.Errorf("error finding existing %s: %w", reflect.TypeFor[T]().Name(), ErrorMultipleMatches)
}

if len(selector) > 0 {
Expand Down Expand Up @@ -92,12 +93,3 @@ func (fctx *FlowContext) defaultSecurityGroupName() string {
func (fctx *FlowContext) defaultSharedNetworkName() string {
return fctx.infra.Namespace
}

func (fctx *FlowContext) workerCIDR() string {
s := fctx.config.Networks.Worker
if workers := fctx.config.Networks.Workers; workers != "" {
s = workers
}

return s
}
7 changes: 4 additions & 3 deletions pkg/controller/infrastructure/terraform_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ func (t *TerraformReconciler) delete(ctx context.Context, infra *extensionsv1alp
destroyKubernetesRoutes = g.Add(flow.Task{
Name: "Destroying Kubernetes route entries",
Fn: flow.TaskFn(func(ctx context.Context) error {
return t.cleanupKubernetesRoutes(ctx, config, networkingClient, vars[infrastructure.TerraformOutputKeyRouterID])
return t.cleanupKubernetesRoutes(ctx, config, networkingClient, vars[infrastructure.TerraformOutputKeyRouterID], vars[infrastructure.TerraformOutputKeySubnetID])
}).RetryUntilTimeout(10*time.Second, 5*time.Minute),
SkipIf: !configExists,
SkipIf: !configExists || (config.Networks.SubnetID != nil && config.Networks.Router != nil),
})

_ = g.Add(flow.Task{
Expand Down Expand Up @@ -231,6 +231,7 @@ func (t *TerraformReconciler) cleanupKubernetesRoutes(
config *api.InfrastructureConfig,
client openstackclient.Networking,
routerID string,
subnetID string,
) error {
if routerID == "" {
return nil
Expand All @@ -239,7 +240,7 @@ func (t *TerraformReconciler) cleanupKubernetesRoutes(
if workesCIDR == "" {
return nil
}
return infrastructure.CleanupKubernetesRoutes(ctx, client, routerID, workesCIDR)
return infrastructure.CleanupKubernetesRoutes(ctx, client, routerID, subnetID)
}

func (t *TerraformReconciler) cleanupKubernetesLoadbalancers(
Expand Down
13 changes: 10 additions & 3 deletions pkg/internal/infrastructure/infrastucture.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,25 @@ func CleanupKubernetesLoadbalancers(ctx context.Context, log logr.Logger, client
}

// CleanupKubernetesRoutes deletes all routes from the router which have a nextHop in the subnet.
func CleanupKubernetesRoutes(_ context.Context, client openstackclient.Networking, routerID, workers string) error {
func CleanupKubernetesRoutes(_ context.Context, client openstackclient.Networking, routerID, subnetID string) error {
router, err := client.GetRouterByID(routerID)
if err != nil {
return err
}

if router == nil {
return nil
}

subnet, err := client.GetSubnetByID(subnetID)
if err != nil {
return err
}
if subnet == nil {
return nil
}

routes := []routers.Route{}
_, workersNet, err := net.ParseCIDR(workers)
_, workersNet, err := net.ParseCIDR(subnet.CIDR)
if err != nil {
return err
}
Expand Down
20 changes: 10 additions & 10 deletions test/integration/infrastructure/infrastructure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ var _ = Describe("Infrastructure tests", func() {
})

It("minimum configuration infrastructure", func() {
providerConfig := newProviderConfig("", nil, nil)
providerConfig := newProviderConfig(nil, nil, nil)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)
namespace, err := generateNamespaceName()
Expect(err).NotTo(HaveOccurred())
Expand All @@ -253,7 +253,7 @@ var _ = Describe("Infrastructure tests", func() {
framework.RemoveCleanupAction(cleanupHandle)
})

providerConfig := newProviderConfig(*routerID, nil, nil)
providerConfig := newProviderConfig(routerID, nil, nil)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)

err = runTest(ctx, log, c, namespace, providerConfig, decoder, cloudProfileConfig)
Expand All @@ -277,7 +277,7 @@ var _ = Describe("Infrastructure tests", func() {
framework.RemoveCleanupAction(cleanupHandle)
})

providerConfig := newProviderConfig("", networkID, nil)
providerConfig := newProviderConfig(nil, networkID, nil)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)

err = runTest(ctx, log, c, namespace, providerConfig, decoder, cloudProfileConfig)
Expand Down Expand Up @@ -310,7 +310,7 @@ var _ = Describe("Infrastructure tests", func() {
framework.RemoveCleanupAction(cleanupHandle)
})

providerConfig := newProviderConfig(*routerID, networkID, nil)
providerConfig := newProviderConfig(routerID, networkID, nil)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)

err = runTest(ctx, log, c, namespace, providerConfig, decoder, cloudProfileConfig)
Expand Down Expand Up @@ -354,14 +354,14 @@ var _ = Describe("Infrastructure tests", func() {
framework.RemoveCleanupAction(cleanupHandle)
})

providerConfig := newProviderConfig(*routerID, subnetID, networkID)
providerConfig := newProviderConfig(routerID, networkID, subnetID)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)

err = runTest(ctx, log, c, namespace, providerConfig, decoder, cloudProfileConfig)
Expect(err).NotTo(HaveOccurred())
})

FIt("with infrastructure that uses existing network and subnet", func() {
It("with infrastructure that uses existing network and subnet", func() {
namespace, err := generateNamespaceName()
Expect(err).NotTo(HaveOccurred())

Expand All @@ -385,7 +385,7 @@ var _ = Describe("Infrastructure tests", func() {
framework.RemoveCleanupAction(cleanupHandle)
})

providerConfig := newProviderConfig("", subnetID, networkID)
providerConfig := newProviderConfig(nil, networkID, subnetID)
cloudProfileConfig := newCloudProfileConfig(*region, *authURL)

err = runTest(ctx, log, c, namespace, providerConfig, decoder, cloudProfileConfig)
Expand Down Expand Up @@ -599,11 +599,11 @@ func checkOperationAnnotationRemoved(obj client.Object) error {
// newProviderConfig creates a providerConfig with the network and router details.
// If routerID is set to "", it requests a new router creation.
// Else it reuses the supplied routerID.
func newProviderConfig(routerID string, networkID *string, subnetID *string) *openstackv1alpha1.InfrastructureConfig {
func newProviderConfig(routerID *string, networkID *string, subnetID *string) *openstackv1alpha1.InfrastructureConfig {
var router *openstackv1alpha1.Router

if routerID != "" {
router = &openstackv1alpha1.Router{ID: routerID}
if routerID != nil {
router = &openstackv1alpha1.Router{ID: *routerID}
}

return &openstackv1alpha1.InfrastructureConfig{
Expand Down

0 comments on commit a111f7b

Please sign in to comment.