Add MS SQL Server Arbiter APIs

apis/helpers.go

@@ -43,10 +43,10 @@ func SetDefaultResourceLimits(req *core.ResourceRequirements, defaultResources c
 	//		- return limit
 	// else
 	//		- return default
+
 	calLimit := func(name core.ResourceName, defaultValue resource.Quantity) resource.Quantity {
 		if r, ok := req.Requests[name]; ok {
-			// l is greater than r == 1.
-			if l, exist := req.Limits[name]; exist && l.Cmp(r) == 1 {
+			if l, exist := req.Limits[name]; exist && l.Cmp(r) > 0 {
 				return l
 			}
 			return r
@@ -56,6 +56,7 @@ func SetDefaultResourceLimits(req *core.ResourceRequirements, defaultResources c
 		}
 		return defaultValue
 	}
+
 	// if request is not set,
 	//		- if limit exists:
 	//				- copy limit
@@ -64,15 +65,15 @@ func SetDefaultResourceLimits(req *core.ResourceRequirements, defaultResources c
 	// else
 	// 		- return request
 	// endif
-	calRequest := func(name core.ResourceName, defaultValue resource.Quantity) resource.Quantity {
-		if r, ok := req.Requests[name]; !ok {
-			if l, exist := req.Limits[name]; exist {
-				return l
-			}
-			return defaultValue
-		} else {
+	calRequest := func(name core.ResourceName, defaultValue resource.Quantity, originalLimit resource.Quantity) resource.Quantity {
+		if r, ok := req.Requests[name]; ok {
 			return r
 		}
+		if originalLimit.Value() > 0 {
+			// If original Limits existed, use them for Requests
+			return originalLimit
+		}
+		return defaultValue
 	}
 
 	if req.Limits == nil {
@@ -82,13 +83,19 @@ func SetDefaultResourceLimits(req *core.ResourceRequirements, defaultResources c
 		req.Requests = core.ResourceList{}
 	}
 
-	// Calculate the limits first
+	// Store the original Limits to differentiate between newly set and existing values
+	originalLimits := make(map[core.ResourceName]resource.Quantity)
+	for l := range req.Limits {
+		originalLimits[l] = req.Limits[l]
+	}
+
+	// Calculate limits first
 	for l := range defaultResources.Limits {
 		req.Limits[l] = calLimit(l, defaultResources.Limits[l])
 	}
 
-	// Once the limit is calculated, Calculate requests
+	// Calculate requests after limits
 	for r := range defaultResources.Requests {
-		req.Requests[r] = calRequest(r, defaultResources.Requests[r])
+		req.Requests[r] = calRequest(r, defaultResources.Requests[r], originalLimits[r])
 	}
 }

apis/helpers_test.go

@@ -0,0 +1,203 @@
+/*
+Copyright AppsCode Inc. and Contributors
+
+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 apis
+
+import (
+	"testing"
+
+	core "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+)
+
+var DefaultResources = core.ResourceRequirements{
+	Requests: core.ResourceList{
+		core.ResourceCPU:    resource.MustParse("1"),
+		core.ResourceMemory: resource.MustParse("2Gi"),
+	},
+	Limits: core.ResourceList{
+		core.ResourceCPU:    resource.MustParse("2"),
+		core.ResourceMemory: resource.MustParse("4Gi"),
+	},
+}
+
+func TestSetDefaultResourceLimits(t *testing.T) {
+	type args struct {
+		req              *core.ResourceRequirements
+		defaultResources core.ResourceRequirements
+	}
+	tests := []struct {
+		name string
+		args args
+	}{
+		{
+			name: "Both the requests and limits are set",
+			args: args{
+				req: &core.ResourceRequirements{
+					Requests: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse(".500"),
+						core.ResourceMemory: resource.MustParse("1Gi"),
+					},
+					Limits: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse("1"),
+						core.ResourceMemory: resource.MustParse("2Gi"),
+					},
+				},
+				defaultResources: DefaultResources,
+			},
+		},
+		{
+			name: "Only requests are set - limits should be set from requests",
+			args: args{
+				req: &core.ResourceRequirements{
+					Requests: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse(".500"),
+						core.ResourceMemory: resource.MustParse("1Gi"),
+					},
+				},
+				defaultResources: DefaultResources,
+			},
+		},
+		{
+			name: "Only limits are set - requests should be set from limits",
+			args: args{
+				req: &core.ResourceRequirements{
+					Limits: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse("1"),
+						core.ResourceMemory: resource.MustParse("2Gi"),
+					},
+				},
+				defaultResources: DefaultResources,
+			},
+		},
+		{
+			name: "Nothing is set - should use default values",
+			args: args{
+				req:              &core.ResourceRequirements{},
+				defaultResources: DefaultResources,
+			},
+		},
+		{
+			name: "Request is greater than limit",
+			args: args{
+				req: &core.ResourceRequirements{
+					Requests: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse("300m"),
+						core.ResourceMemory: resource.MustParse("300Mi"),
+					},
+					Limits: core.ResourceList{
+						core.ResourceCPU:    resource.MustParse("200m"),
+						core.ResourceMemory: resource.MustParse("200Mi"),
+					},
+				},
+				defaultResources: DefaultResources,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			old := tt.args.req
+			SetDefaultResourceLimits(tt.args.req, tt.args.defaultResources)
+			if !checkExpected(tt.args.req, old) {
+				t.Errorf("Expected SetDefaultResourceLimits to set default limits correctly")
+			}
+		})
+	}
+}
+
+func checkExpected(req *core.ResourceRequirements, old *core.ResourceRequirements) bool {
+	// Check if requests and limits are properly initialized
+	if req.Requests == nil || req.Limits == nil {
+		return false
+	}
+
+	// If both requests and limits exist, verify values are preserved or adjusted as needed
+	if old != nil && old.Requests != nil && old.Limits != nil {
+		for name, oldReq := range old.Requests {
+			oldLim, limExists := old.Limits[name]
+			if limExists {
+				// verify requests is preserved
+				if newReq, exists := req.Requests[name]; !exists || newReq.Cmp(oldReq) != 0 {
+					return false
+				}
+				if oldReq.Cmp(oldLim) > 0 {
+					// Request is greater than limit, so limit should be set to request
+					if newLim, exists := req.Limits[name]; !exists || newLim.Cmp(oldReq) != 0 {
+						return false
+					}
+				} else {
+					// verify limit is preserved
+					if newLim, exists := req.Limits[name]; !exists || newLim.Cmp(oldLim) != 0 {
+						return false
+					}
+				}
+			}
+		}
+	}
+
+	// If old requests existed but no limits, verify limits are set from requests
+	if old != nil && old.Requests != nil && len(old.Limits) == 0 {
+		for name, oldReq := range old.Requests {
+			if newLim, exists := req.Limits[name]; !exists || newLim.Cmp(oldReq) != 0 {
+				return false
+			}
+			// Also verify that request value is preserved
+			if newReq, exists := req.Requests[name]; !exists || newReq.Cmp(oldReq) != 0 {
+				return false
+			}
+		}
+	}
+
+	// If old limits existed but no requests, verify requests are set from limits
+	if old != nil && old.Limits != nil && len(old.Requests) == 0 {
+		for name, oldLim := range old.Limits {
+			if newReq, exists := req.Requests[name]; !exists || newReq.Cmp(oldLim) != 0 {
+				return false
+			}
+			// Also verify that limit value is preserved
+			if newLim, exists := req.Limits[name]; !exists || newLim.Cmp(oldLim) != 0 {
+				return false
+			}
+		}
+	}
+
+	// If neither requests nor limits existed, verify default values are used
+	if old != nil && len(old.Requests) == 0 && len(old.Limits) == 0 {
+		// CPU check
+		if cpuReq, exists := req.Requests[core.ResourceCPU]; !exists || cpuReq.Cmp(*DefaultResources.Requests.Cpu()) != 0 {
+			return false
+		}
+		if cpuLim, exists := req.Limits[core.ResourceCPU]; !exists || cpuLim.Cmp(*DefaultResources.Limits.Cpu()) != 0 {
+			return false
+		}
+		// Memory check
+		if memReq, exists := req.Requests[core.ResourceMemory]; !exists || memReq.Cmp(*DefaultResources.Requests.Memory()) != 0 {
+			return false
+		}
+		if memLim, exists := req.Limits[core.ResourceMemory]; !exists || memLim.Cmp(*DefaultResources.Limits.Memory()) != 0 {
+			return false
+		}
+	}
+
+	// For all cases, ensure limits are not less than requests
+	for name, reqVal := range req.Requests {
+		if limVal, exists := req.Limits[name]; !exists || limVal.Cmp(reqVal) < 0 {
+			return false
+		}
+	}
+
+	return true
+}

apis/kubedb/constants.go

@@ -416,9 +416,12 @@ const (
 	MSSQLDatabasePortName              = "db"
 	MSSQLPrimaryServicePortName        = "primary"
 	MSSQLSecondaryServicePortName      = "secondary"
+	MSSQLCoordinatorPortName           = "coordinator"
+	MSSQLCoordinatorClientPortName     = "coordinatclient"
 	MSSQLDatabasePort                  = 1433
 	MSSQLDatabaseMirroringEndpointPort = 5022
-	MSSQLCoordinatorPort               = 2381
+	MSSQLCoordinatorPort               = 2380
+	MSSQLCoordinatorClientPort         = 2379
 	MSSQLMonitoringDefaultServicePort  = 9399
 
 	// environment variables
@@ -1695,7 +1698,7 @@ var (
 			core.ResourceMemory: resource.MustParse("1.5Gi"),
 		},
 		Limits: core.ResourceList{
-			core.ResourceMemory: resource.MustParse("4Gi"),
+			core.ResourceMemory: resource.MustParse("2Gi"),
 		},
 	}
 

apis/kubedb/v1alpha2/mssqlserver_helpers.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/ptr"
 	kmapi "kmodules.xyz/client-go/api/v1"
 	"kmodules.xyz/client-go/apiextensions"
 	coreutil "kmodules.xyz/client-go/core/v1"
@@ -383,6 +384,8 @@ func (m *MSSQLServer) SetDefaults() {
 		return
 	}
 
+	m.SetArbiterDefault()
+
 	m.setDefaultContainerSecurityContext(&mssqlVersion, m.Spec.PodTemplate)
 
 	m.SetTLSDefaults()
@@ -509,6 +512,15 @@ func (m *MSSQLServer) setDefaultContainerResourceLimits(podTemplate *ofst.PodTem
 	}
 }
 
+func (m *MSSQLServer) SetArbiterDefault() {
+	if m.IsAvailabilityGroup() && ptr.Deref(m.Spec.Replicas, 0)%2 == 0 && m.Spec.Arbiter == nil {
+		m.Spec.Arbiter = &ArbiterSpec{
+			Resources: core.ResourceRequirements{},
+		}
+		apis.SetDefaultResourceLimits(&m.Spec.Arbiter.Resources, kubedb.DefaultArbiter(false))
+	}
+}
+
 func (m *MSSQLServer) SetTLSDefaults() {
 	if m.Spec.TLS == nil || m.Spec.TLS.IssuerRef == nil {
 		return

apis/kubedb/v1alpha2/mssqlserver_types.go

@@ -130,6 +130,10 @@ type MSSQLServerSpec struct {
 	// Archiver controls database backup using Archiver CR
 	// +optional
 	Archiver *Archiver `json:"archiver,omitempty"`
+
+	// Arbiter controls spec for arbiter pods
+	// +optional
+	Arbiter *ArbiterSpec `json:"arbiter,omitempty"`
 }
 
 type MSSQLServerTLSConfig struct {

apis/kubedb/v1alpha2/postgres_types.go

@@ -188,21 +188,6 @@ type PostgresReplication struct {
 	MaxSlotWALKeepSizeInMegaBytes *int32 `json:"maxSlotWALKeepSize,omitempty"`
 }
 
-type ArbiterSpec struct {
-	// Compute Resources required by the sidecar container.
-	// +optional
-	Resources core.ResourceRequirements `json:"resources,omitempty"`
-	// NodeSelector is a selector which must be true for the pod to fit on a node.
-	// Selector which must match a node's labels for the pod to be scheduled on that node.
-	// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
-	// +optional
-	// +mapType=atomic
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
-	// If specified, the pod's tolerations.
-	// +optional
-	Tolerations []core.Toleration `json:"tolerations,omitempty"`
-}
-
 // PostgreLeaderElectionConfig contains essential attributes of leader election.
 type PostgreLeaderElectionConfig struct {
 	// LeaseDuration is the duration in second that non-leader candidates will