From d7513e3cb911171f0229448db65cd9bdab47147f Mon Sep 17 00:00:00 2001 From: Omer Schwartz Date: Fri, 20 Sep 2024 12:11:51 +0000 Subject: [PATCH 1/2] Add secret hash to worker hash So far we had the rndc secret mounted on the worker pods, but any change to the existing secret would not modify the mounted rndc keys. This patch adds the secret's hash to the worker's hash, which will "synchronize" the mounted keys according to the secret's state. --- controllers/designateworker_controller.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/controllers/designateworker_controller.go b/controllers/designateworker_controller.go index 613cd163..cb7d4893 100644 --- a/controllers/designateworker_controller.go +++ b/controllers/designateworker_controller.go @@ -487,7 +487,7 @@ func (r *DesignateWorkerReconciler) reconcileNormal(ctx context.Context, instanc // create hash over all the different input resources to identify if any those changed // and a restart/recreate is required. // - inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, instance, configMapVars) + inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, helper, instance, configMapVars) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, @@ -798,12 +798,30 @@ func (r *DesignateWorkerReconciler) generateServiceConfigMaps( // returns the hash, whether the hash changed (as a bool) and any error func (r *DesignateWorkerReconciler) createHashOfInputHashes( ctx context.Context, + h *helper.Helper, instance *designatev1beta1.DesignateWorker, envVars map[string]env.Setter, ) (string, bool, error) { Log := r.GetLogger(ctx) var hashMap map[string]string changed := false + + // If DesignateBindKeySecret exists, add its hash to status hash + rndcSecret := &corev1.Secret{} + err := h.GetClient().Get(ctx, types.NamespacedName{ + Name: designate.DesignateBindKeySecret, + Namespace: instance.Namespace, + }, rndcSecret) + if err != nil { + Log.Error(err, "Unable to retrieve rndc key secret") + return "", false, err + } + secretHash, err := secret.Hash(rndcSecret) + if err != nil { + return secretHash, changed, err + } + + envVars[designate.DesignateBindKeySecret] = env.SetValue(secretHash) mergedMapVars := env.MergeEnvs([]corev1.EnvVar{}, envVars) hash, err := util.ObjectHash(mergedMapVars) if err != nil { From 8cd5badb311f6f6a6a03b2f6bac160d5c64255fa Mon Sep 17 00:00:00 2001 From: Omer Schwartz Date: Wed, 25 Sep 2024 14:41:03 +0000 Subject: [PATCH 2/2] Implement predictable IP generation for miniDNS The designate miniDNS needs a predictable IP mechanism, as it is a daemonset and will use the list of available workers to generate a list of IPs that are valid for the designate network attachment's CIDR range but do not overlap with the address range defined in the attachment's IPAM config. This patch adds it. --- .../designate.openstack.org_designates.yaml | 6 + api/v1beta1/designate_types.go | 5 + .../designate.openstack.org_designates.yaml | 6 + config/rbac/role.yaml | 7 ++ controllers/designate_controller.go | 107 ++++++++++++++++++ controllers/designatemdns_controller.go | 20 +++- pkg/designate/bind_ctrl_network.go | 56 +++++++++ pkg/designate/const.go | 2 + pkg/designate/network_consts.go | 26 +++++ pkg/designate/network_parameters.go | 87 ++++++++++++++ pkg/designatemdns/daemonset.go | 10 +- pkg/designatemdns/volumes.go | 49 ++++++++ 12 files changed, 373 insertions(+), 8 deletions(-) create mode 100644 pkg/designate/bind_ctrl_network.go create mode 100644 pkg/designate/network_consts.go create mode 100644 pkg/designate/network_parameters.go create mode 100644 pkg/designatemdns/volumes.go diff --git a/api/bases/designate.openstack.org_designates.yaml b/api/bases/designate.openstack.org_designates.yaml index 7abe4087..d0f65906 100644 --- a/api/bases/designate.openstack.org_designates.yaml +++ b/api/bases/designate.openstack.org_designates.yaml @@ -940,6 +940,11 @@ spec: required: - containerImage type: object + designateNetworkAttachment: + default: designate + description: DesignateNetworkAttachment is a NetworkAttachment resource + name for the Designate Control Network + type: string designateProducer: description: DesignateProducer - Spec definition for the Producer service of this Designate deployment @@ -1473,6 +1478,7 @@ spec: - designateBackendbind9 - designateCentral - designateMdns + - designateNetworkAttachment - designateProducer - designateWorker - rabbitMqClusterName diff --git a/api/v1beta1/designate_types.go b/api/v1beta1/designate_types.go index 85758650..3d9c3853 100644 --- a/api/v1beta1/designate_types.go +++ b/api/v1beta1/designate_types.go @@ -176,6 +176,11 @@ type DesignateSpecBase struct { // Resources - Compute Resources required by this service (Limits/Requests). // https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ Resources corev1.ResourceRequirements `json:"resources,omitempty"` + + // +kubebuilder:validation:Required + // +kubebuilder:default=designate + // DesignateNetworkAttachment is a NetworkAttachment resource name for the Designate Control Network + DesignateNetworkAttachment string `json:"designateNetworkAttachment"` } // DesignateStatus defines the observed state of Designate diff --git a/config/crd/bases/designate.openstack.org_designates.yaml b/config/crd/bases/designate.openstack.org_designates.yaml index 7abe4087..d0f65906 100644 --- a/config/crd/bases/designate.openstack.org_designates.yaml +++ b/config/crd/bases/designate.openstack.org_designates.yaml @@ -940,6 +940,11 @@ spec: required: - containerImage type: object + designateNetworkAttachment: + default: designate + description: DesignateNetworkAttachment is a NetworkAttachment resource + name for the Designate Control Network + type: string designateProducer: description: DesignateProducer - Spec definition for the Producer service of this Designate deployment @@ -1473,6 +1478,7 @@ spec: - designateBackendbind9 - designateCentral - designateMdns + - designateNetworkAttachment - designateProducer - designateWorker - rabbitMqClusterName diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 28b3f956..362f9496 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -5,6 +5,13 @@ metadata: creationTimestamp: null name: manager-role rules: +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list - apiGroups: - "" resources: diff --git a/controllers/designate_controller.go b/controllers/designate_controller.go index b6c1b85c..fa680b74 100644 --- a/controllers/designate_controller.go +++ b/controllers/designate_controller.go @@ -115,6 +115,7 @@ type DesignateReconciler struct { // +kubebuilder:rbac:groups=keystone.openstack.org,resources=keystoneapis,verbs=get;list;watch // +kubebuilder:rbac:groups=rabbitmq.openstack.org,resources=transporturls,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=nodes,verbs=get;list // service account, role, rolebinding // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch @@ -713,6 +714,112 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des } Log.Info("Deployment API task reconciled") + nad, err := nad.GetNADWithName(ctx, helper, instance.Spec.DesignateNetworkAttachment, instance.Namespace) + if err != nil { + return ctrl.Result{}, err + } + + networkParameters, err := designate.GetNetworkParametersFromNAD(nad) + if err != nil { + return ctrl.Result{}, err + } + + nodeConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: designate.MdnsPredIPConfigMap, + Namespace: instance.GetNamespace(), + Labels: labels.GetLabels(instance, labels.GetGroupLabel(instance.ObjectMeta.Name), map[string]string{}), + }, + Data: make(map[string]string), + } + + // Look for existing config map and if exists, read existing data and match + // against nodes. + foundMap := &corev1.ConfigMap{} + err = helper.GetClient().Get(ctx, types.NamespacedName{Name: designate.MdnsPredIPConfigMap, Namespace: instance.GetNamespace()}, + foundMap) + if err != nil { + if k8s_errors.IsNotFound(err) { + Log.Info(fmt.Sprintf("Ip map %s doesn't exist, creating.", designate.MdnsPredIPConfigMap)) + } else { + return ctrl.Result{}, err + } + } else { + Log.Info("Retrieved existing map, updating..") + nodeConfigMap.Data = foundMap.Data + } + + // + // Predictable IPs. + // + // NOTE(oschwart): refactoring this might be nice. This could also be + // optimized but the data sets are small (nodes an IP ranges are less than + // 100) so optimization might be a waste. + // + predictableIPParams, err := designate.GetPredictableIPAM(networkParameters) + if err != nil { + return ctrl.Result{}, err + } + // Get a list of the nodes in the cluster + + // TODO(oschwart): + // * confirm whether or not this lists only the nodes we want (i.e. ones + // that will host the daemonset) + // * do we want to provide a mechanism to temporarily disabling this list + // for maintenance windows where nodes might be "coming and going" + + nodes, err := helper.GetKClient().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + return ctrl.Result{}, err + } + updatedMap := make(map[string]string) + allocatedIPs := make(map[string]bool) + var predictableIPsRequired []string + + // First scan existing allocations so we can keep existing allocations. + // Keeping track of what's required and what already exists. If a node is + // removed from the cluster, it's IPs will not be added to the allocated + // list and are effectively recycled. + for _, node := range nodes.Items { + nodeName := fmt.Sprintf("mdns_%s", node.Name) + if ipValue, ok := nodeConfigMap.Data[nodeName]; ok { + updatedMap[nodeName] = ipValue + allocatedIPs[ipValue] = true + Log.Info(fmt.Sprintf("%s has IP mapping %s: %s", node.Name, nodeName, ipValue)) + } else { + predictableIPsRequired = append(predictableIPsRequired, nodeName) + } + } + // Get new IPs using the range from predictableIPParmas minus the + // allocatedIPs captured above. + Log.Info(fmt.Sprintf("Allocating %d predictable IPs", len(predictableIPsRequired))) + for _, nodeName := range predictableIPsRequired { + nodeIP, err := designate.GetNextIP(predictableIPParams, allocatedIPs) + if err != nil { + // An error here is really unexpected- it means either we have + // messed up the allocatedIPs list or the range we are assuming is + // too small for the number of mdns pod. + return ctrl.Result{}, err + } + updatedMap[nodeName] = nodeIP + } + + mapLabels := labels.GetLabels(instance, labels.GetGroupLabel(instance.ObjectMeta.Name), map[string]string{}) + _, err = controllerutil.CreateOrPatch(ctx, helper.GetClient(), nodeConfigMap, func() error { + nodeConfigMap.Labels = util.MergeStringMaps(nodeConfigMap.Labels, mapLabels) + nodeConfigMap.Data = updatedMap + err := controllerutil.SetControllerReference(instance, nodeConfigMap, helper.GetScheme()) + if err != nil { + return err + } + return nil + }) + + if err != nil { + Log.Info("Unable to create config map for mdns ips...") + return ctrl.Result{}, err + } + // deploy designate-central designateCentral, op, err := r.centralDeploymentCreateOrUpdate(ctx, instance) if err != nil { diff --git a/controllers/designatemdns_controller.go b/controllers/designatemdns_controller.go index f141c019..23c2f631 100644 --- a/controllers/designatemdns_controller.go +++ b/controllers/designatemdns_controller.go @@ -43,6 +43,7 @@ import ( designatemdns "github.com/openstack-k8s-operators/designate-operator/pkg/designatemdns" "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/configmap" "github.com/openstack-k8s-operators/lib-common/modules/common/daemonset" "github.com/openstack-k8s-operators/lib-common/modules/common/env" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" @@ -489,7 +490,7 @@ func (r *DesignateMdnsReconciler) reconcileNormal(ctx context.Context, instance // create hash over all the different input resources to identify if any those changed // and a restart/recreate is required. // - inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, instance, configMapVars) + inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, helper, instance, configMapVars) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, @@ -803,6 +804,7 @@ func (r *DesignateMdnsReconciler) generateServiceConfigMaps( // returns the hash, whether the hash changed (as a bool) and any error func (r *DesignateMdnsReconciler) createHashOfInputHashes( ctx context.Context, + h *helper.Helper, instance *designatev1beta1.DesignateMdns, envVars map[string]env.Setter, ) (string, bool, error) { @@ -810,6 +812,22 @@ func (r *DesignateMdnsReconciler) createHashOfInputHashes( var hashMap map[string]string changed := false + + // If MdnsPredIPConfigMap exists, add its hash to status hash + mdnsPredIPCM := &corev1.ConfigMap{} + err := h.GetClient().Get(ctx, types.NamespacedName{ + Name: designate.MdnsPredIPConfigMap, + Namespace: instance.Namespace, + }, mdnsPredIPCM) + if err != nil { + Log.Error(err, "Unable to retrieve Mdns predictable IPs ConfigMap") + return "", false, err + } + mdnsPredIPCMHash, err := configmap.Hash(mdnsPredIPCM) + if err != nil { + return mdnsPredIPCMHash, changed, err + } + mergedMapVars := env.MergeEnvs([]corev1.EnvVar{}, envVars) hash, err := util.ObjectHash(mergedMapVars) if err != nil { diff --git a/pkg/designate/bind_ctrl_network.go b/pkg/designate/bind_ctrl_network.go new file mode 100644 index 00000000..5c696d5e --- /dev/null +++ b/pkg/designate/bind_ctrl_network.go @@ -0,0 +1,56 @@ +/* +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 designate + +import ( + "fmt" +) + +// GetPredictableIPAM returns a struct describing the available IP range. If the +// IP pool size does not fit in given networkParameters CIDR it will return an +// error instead. +func GetPredictableIPAM(networkParameters *NetworkParameters) (*NADIpam, error) { + predParams := &NADIpam{} + predParams.CIDR = networkParameters.CIDR + predParams.RangeStart = networkParameters.ProviderAllocationEnd.Next() + endRange := predParams.RangeStart + for i := 0; i < BindProvPredictablePoolSize; i++ { + if !predParams.CIDR.Contains(endRange) { + return nil, fmt.Errorf("predictable IPs: cannot allocate %d IP addresses in %s", BindProvPredictablePoolSize, predParams.CIDR) + } + endRange = endRange.Next() + } + predParams.RangeEnd = endRange + return predParams, nil +} + +// GetNextIP picks the next available IP from the range defined by a NADIpam, +// skipping ones that are already used appear as keys in the currentValues map. +func GetNextIP(predParams *NADIpam, currentValues map[string]bool) (string, error) { + candidateAddress := predParams.RangeStart + for alloced := true; alloced; { + + if _, ok := currentValues[candidateAddress.String()]; ok { + if candidateAddress == predParams.RangeEnd { + return "", fmt.Errorf("predictable IPs: out of available addresses") + } + candidateAddress = candidateAddress.Next() + } else { + alloced = false + } + } + currentValues[candidateAddress.String()] = true + return candidateAddress.String(), nil +} diff --git a/pkg/designate/const.go b/pkg/designate/const.go index 36845f26..108d4cea 100644 --- a/pkg/designate/const.go +++ b/pkg/designate/const.go @@ -38,4 +38,6 @@ const ( DesignateBindKeySecret = "designate-bind-secret" DesignateRndcKey = "rndc-key" + + MdnsPredIPConfigMap = "designate-mdns-ip-map" ) diff --git a/pkg/designate/network_consts.go b/pkg/designate/network_consts.go new file mode 100644 index 00000000..ba645834 --- /dev/null +++ b/pkg/designate/network_consts.go @@ -0,0 +1,26 @@ +/* + +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 designate + +// NOTE: Strictly speaking, these don't have to be package scope constants, but having them externally +// accessible might aide constructing functional tests later on. + +const ( + // Common consts for Control network + + // BindProvPredictablePoolSize - + BindProvPredictablePoolSize = 25 +) diff --git a/pkg/designate/network_parameters.go b/pkg/designate/network_parameters.go new file mode 100644 index 00000000..44741a35 --- /dev/null +++ b/pkg/designate/network_parameters.go @@ -0,0 +1,87 @@ +/* +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 designate + +import ( + "encoding/json" + "fmt" + "net/netip" + + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" +) + +// NetworkParameters - Parameters for the Designate networks, based on the config of the NAD +type NetworkParameters struct { + CIDR netip.Prefix + ProviderAllocationStart netip.Addr + ProviderAllocationEnd netip.Addr +} + +// NADConfig - IPAM parameters of the NAD +type NADConfig struct { + IPAM NADIpam `json:"ipam"` +} + +type NADIpam struct { + CIDR netip.Prefix `json:"range"` + RangeStart netip.Addr `json:"range_start"` + RangeEnd netip.Addr `json:"range_end"` +} + +func getConfigFromNAD( + nad *networkv1.NetworkAttachmentDefinition, +) (*NADConfig, error) { + nadConfig := &NADConfig{} + jsonDoc := []byte(nad.Spec.Config) + err := json.Unmarshal(jsonDoc, nadConfig) + if err != nil { + return nil, err + } + + return nadConfig, nil +} + +// GetNetworkParametersFromNAD - Extract network information from the Network Attachment Definition +func GetNetworkParametersFromNAD( + nad *networkv1.NetworkAttachmentDefinition, +) (*NetworkParameters, error) { + networkParameters := &NetworkParameters{} + + nadConfig, err := getConfigFromNAD(nad) + if err != nil { + return nil, fmt.Errorf("cannot read network parameters: %w", err) + } + + // Designate CIDR parameters + // These are the parameters for Designate's net/subnet + networkParameters.CIDR = nadConfig.IPAM.CIDR + + // OpenShift allocates IP addresses from IPAM.RangeStart to IPAM.RangeEnd + // for the pods. + // We're going to use a range of 25 IP addresses that are assigned to + // the Neutron allocation pool, the range starts right after OpenShift + // RangeEnd. + networkParameters.ProviderAllocationStart = nadConfig.IPAM.RangeEnd.Next() + end := networkParameters.ProviderAllocationStart + for i := 0; i < BindProvPredictablePoolSize; i++ { + if !networkParameters.CIDR.Contains(end) { + return nil, fmt.Errorf("cannot allocate %d IP addresses in %s", BindProvPredictablePoolSize, networkParameters.CIDR) + } + end = end.Next() + } + networkParameters.ProviderAllocationEnd = end + + return networkParameters, err +} diff --git a/pkg/designatemdns/daemonset.go b/pkg/designatemdns/daemonset.go index 3e9b3111..a0245cdc 100644 --- a/pkg/designatemdns/daemonset.go +++ b/pkg/designatemdns/daemonset.go @@ -43,11 +43,9 @@ func DaemonSet( annotations map[string]string, ) *appsv1.DaemonSet { rootUser := int64(0) - - volumes := designate.GetVolumes( - designate.GetOwningDesignateName(instance), - ) - volumeMounts := designate.GetVolumeMounts("designate-mdns") + serviceName := fmt.Sprintf("%s-mdns", designate.ServiceName) + volumes := GetVolumes(designate.GetOwningDesignateName(instance)) + volumeMounts := GetVolumeMounts(serviceName) livenessProbe := &corev1.Probe{ // TODO might need tuning @@ -74,8 +72,6 @@ func DaemonSet( envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS") envVars["CONFIG_HASH"] = env.SetValue(configHash) - serviceName := fmt.Sprintf("%s-mdns", designate.ServiceName) - // Add the CA bundle if instance.Spec.TLS.CaBundleSecretName != "" { volumes = append(volumes, instance.Spec.TLS.CreateVolume()) diff --git a/pkg/designatemdns/volumes.go b/pkg/designatemdns/volumes.go new file mode 100644 index 00000000..ef40f5c3 --- /dev/null +++ b/pkg/designatemdns/volumes.go @@ -0,0 +1,49 @@ +/* +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 designatemdns + +import ( + corev1 "k8s.io/api/core/v1" + + "github.com/openstack-k8s-operators/designate-operator/pkg/designate" +) + +func GetVolumes(name string) []corev1.Volume { + var config0640AccessMode int32 = 0640 + return append( + designate.GetVolumes(name), + corev1.Volume{ + Name: "bind-ips", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: designate.MdnsPredIPConfigMap, + }, + DefaultMode: &config0640AccessMode, + }, + }, + }, + ) +} + +func GetVolumeMounts(serviceName string) []corev1.VolumeMount { + return append( + designate.GetVolumeMounts(serviceName), + corev1.VolumeMount{ + Name: "bind-ips", + MountPath: "/var/lib/bind-ips", + ReadOnly: true, + }, + ) +}