diff --git a/pkg/api/galaxy/constant/constant.go b/pkg/api/galaxy/constant/constant.go index 748d537d..613df1fc 100644 --- a/pkg/api/galaxy/constant/constant.go +++ b/pkg/api/galaxy/constant/constant.go @@ -114,6 +114,10 @@ func ConvertReleasePolicy(policyStr string) ReleasePolicy { } } +func PolicyStr(policy ReleasePolicy) string { + return [...]string{"", Immutable, Never}[policy] +} + const ( ResourceKind = "FloatingIP" ApiVersion = "galaxy.k8s.io/v1alpha1" diff --git a/pkg/ipam/schedulerplugin/floatingip_plugin.go b/pkg/ipam/schedulerplugin/floatingip_plugin.go index bf467f71..29637c56 100644 --- a/pkg/ipam/schedulerplugin/floatingip_plugin.go +++ b/pkg/ipam/schedulerplugin/floatingip_plugin.go @@ -232,6 +232,9 @@ func (p *FloatingIPPlugin) getSubnet(pod *corev1.Pod) (sets.String, error) { return subnets, nil } policy := parseReleasePolicy(&pod.ObjectMeta) + if !keyObj.Deployment() && !keyObj.StatefulSet() && !keyObj.TApp() && policy != constant.ReleasePolicyPodDelete { + return nil, fmt.Errorf("policy %s not supported for non deployment/tapp/sts app", constant.PolicyStr(policy)) + } var replicas int var isPoolSizeDefined bool if keyObj.Deployment() { diff --git a/pkg/ipam/schedulerplugin/floatingip_plugin_test.go b/pkg/ipam/schedulerplugin/floatingip_plugin_test.go index c99d6f8f..1fc309e4 100644 --- a/pkg/ipam/schedulerplugin/floatingip_plugin_test.go +++ b/pkg/ipam/schedulerplugin/floatingip_plugin_test.go @@ -130,6 +130,21 @@ func TestFilter(t *testing.T) { } } +func TestFilterForPodWithoutRef(t *testing.T) { + fipPlugin, stopChan, nodes := createPluginTestNodes(t) + defer func() { stopChan <- struct{}{} }() + filtered, failed, err := fipPlugin.Filter(CreateSimplePod("pod1", "ns1", nil), nodes) + if err != nil { + t.Fatal(err) + } + if err := checkFilterResult(filtered, failed, []string{node3, node4}, []string{drainedNode, nodeHasNoIP}); err != nil { + t.Fatal(err) + } + if _, _, err = fipPlugin.Filter(CreateSimplePod("pod1", "ns1", immutableAnnotation), nodes); err == nil { + t.Fatalf("expect an error for non sts/deployment/tapp pod with policy immutable") + } +} + func TestAllocateIP(t *testing.T) { fipPlugin, stopChan, _ := createPluginTestNodes(t) defer func() { stopChan <- struct{}{} }() diff --git a/pkg/ipam/schedulerplugin/resync.go b/pkg/ipam/schedulerplugin/resync.go index 1822f1e9..845f2188 100644 --- a/pkg/ipam/schedulerplugin/resync.go +++ b/pkg/ipam/schedulerplugin/resync.go @@ -185,8 +185,8 @@ func (p *FloatingIPPlugin) resyncAllocatedIPs(ipam floatingip.IPAM, meta *resync replicas = tapp.Spec.Replicas } } else { - glog.Warningf("unknow app type of key %s", obj.keyObj.KeyInDB) - continue + // release for other apps + appExist = false } if should, reason := p.shouldReleaseDuringResync(obj.keyObj, releasePolicy, appExist, replicas); should { if err := releaseIP(ipam, key, fmt.Sprintf("%s during resyncing", reason)); err != nil { diff --git a/pkg/ipam/schedulerplugin/testing/util.go b/pkg/ipam/schedulerplugin/testing/util.go index 0e80dc33..cea68424 100644 --- a/pkg/ipam/schedulerplugin/testing/util.go +++ b/pkg/ipam/schedulerplugin/testing/util.go @@ -73,3 +73,16 @@ func CreateTAppPod(name, namespace string, annotations map[string]string) *corev pod.OwnerReferences[0].Kind = "TApp" return pod } + +func CreateSimplePod(name, namespace string, annotations map[string]string) *corev1.Pod { + pod := CreateStatefulSetPod(name+"-0", namespace, annotations) + pod.Name = name + pod.OwnerReferences = nil + return pod +} + +func CreatePodWithKind(name, namespace, kind string, annotations map[string]string) *corev1.Pod { + pod := CreateStatefulSetPod(name, namespace, annotations) + pod.OwnerReferences[0].Kind = kind + return pod +} diff --git a/pkg/ipam/schedulerplugin/util/utils.go b/pkg/ipam/schedulerplugin/util/utils.go index b4bf9e0c..239bf326 100644 --- a/pkg/ipam/schedulerplugin/util/utils.go +++ b/pkg/ipam/schedulerplugin/util/utils.go @@ -122,6 +122,9 @@ const ( DeploymentPrefixKey = "dp_" StatefulsetPrefixKey = "sts_" TAppPrefixKey = "tapp_" + + NoRefAppName = "NULL" + NoRefAppTypePrefix = "NULL_" ) func FormatKey(pod *corev1.Pod) (*KeyObj, error) { @@ -131,22 +134,28 @@ func FormatKey(pod *corev1.Pod) (*KeyObj, error) { PodName: pod.Name, Namespace: pod.Namespace} if len(pod.OwnerReferences) == 0 { - return keyObj, fmt.Errorf("doesn't support pods which does not have parent app") - } - if pod.OwnerReferences[0].Kind == "StatefulSet" { - keyObj.AppName = pod.OwnerReferences[0].Name - keyObj.AppTypePrefix = StatefulsetPrefixKey - } else if pod.OwnerReferences[0].Kind == "TApp" { - keyObj.AppName = pod.OwnerReferences[0].Name - keyObj.AppTypePrefix = TAppPrefixKey + keyObj.AppName = NoRefAppName + keyObj.AppTypePrefix = NoRefAppTypePrefix } else { - deploymentName := resolveDeploymentName(pod) - if deploymentName == "" { - return keyObj, fmt.Errorf("unsupported app type") + if pod.OwnerReferences[0].Kind == "StatefulSet" { + keyObj.AppName = pod.OwnerReferences[0].Name + keyObj.AppTypePrefix = StatefulsetPrefixKey + } else if pod.OwnerReferences[0].Kind == "TApp" { + keyObj.AppName = pod.OwnerReferences[0].Name + keyObj.AppTypePrefix = TAppPrefixKey + } else if pod.OwnerReferences[0].Kind != "ReplicaSet" { + // some app type galaxy can't recognize + keyObj.AppName = pod.OwnerReferences[0].Name + keyObj.AppTypePrefix = strings.ToLower(pod.OwnerReferences[0].Kind) + "_" + } else { + deploymentName := resolveDeploymentName(pod) + if deploymentName == "" { + return keyObj, fmt.Errorf("unsupported app type") + } + keyObj.AppName = deploymentName + // treat rs like deployment, share the same appPrefixKey + keyObj.AppTypePrefix = DeploymentPrefixKey } - keyObj.AppName = deploymentName - // treat rs like deployment, share the same appPrefixKey - keyObj.AppTypePrefix = DeploymentPrefixKey } keyObj.genKey() return keyObj, nil @@ -166,26 +175,19 @@ func ParseKey(key string) *KeyObj { keyObj.PoolName = parts[0] removedPoolKey = parts[1] } - if strings.HasPrefix(removedPoolKey, DeploymentPrefixKey) { - keyObj.AppTypePrefix = DeploymentPrefixKey - } else if strings.HasPrefix(removedPoolKey, StatefulsetPrefixKey) { - keyObj.AppTypePrefix = StatefulsetPrefixKey - } else if strings.HasPrefix(removedPoolKey, TAppPrefixKey) { - keyObj.AppTypePrefix = TAppPrefixKey - } - keyObj.AppName, keyObj.PodName, keyObj.Namespace = resolvePodKey(removedPoolKey) + keyObj.AppTypePrefix, keyObj.AppName, keyObj.PodName, keyObj.Namespace = resolvePodKey(removedPoolKey) return keyObj } // resolvePodKey returns appname, podName, namespace -// "sts_kube-system_fip-bj_fip-bj-111": {"fip-bj", "fip-bj-111", "kube-system"} -func resolvePodKey(key string) (string, string, string) { +// "sts_kube-system_fip-bj_fip-bj-111": {"sts_", "fip-bj", "fip-bj-111", "kube-system"} +func resolvePodKey(key string) (string, string, string, string) { // _ is not a valid char in appname parts := strings.Split(key, "_") if len(parts) == 4 { - return parts[2], parts[3], parts[1] + return parts[0] + "_", parts[2], parts[3], parts[1] } - return "", "", "" + return "", "", "", "" } func Join(name, namespace string) string { diff --git a/pkg/ipam/schedulerplugin/util/utils_test.go b/pkg/ipam/schedulerplugin/util/utils_test.go index 555232e6..99ecf9d7 100644 --- a/pkg/ipam/schedulerplugin/util/utils_test.go +++ b/pkg/ipam/schedulerplugin/util/utils_test.go @@ -28,20 +28,24 @@ import ( func TestResolvePodKey(t *testing.T) { tests := map[string][]string{ - "dp_default_dp1_dp1-rs1-pod1": {"default", "dp1", "dp1-rs1-pod1"}, - "sts_default_fip_fip-0": {"default", "fip", "fip-0"}, - "sts_kube-system_fip-bj_fip-bj-111": {"kube-system", "fip-bj", "fip-bj-111"}, - "tapp_kube-system_tapp-bj_tapp-bj-2091": {"kube-system", "tapp-bj", "tapp-bj-2091"}, + "dp_default_dp1_dp1-rs1-pod1": {"dp_", "default", "dp1", "dp1-rs1-pod1"}, + "sts_default_fip_fip-0": {"sts_", "default", "fip", "fip-0"}, + "sts_kube-system_fip-bj_fip-bj-111": {"sts_", "kube-system", "fip-bj", "fip-bj-111"}, + "tapp_kube-system_tapp-bj_tapp-bj-2091": {"tapp_", "kube-system", "tapp-bj", "tapp-bj-2091"}, + "NULL_default_NULL_pod1": {"NULL_", "default", "NULL", "pod1"}, } for k, v := range tests { - appname, podName, namespace := resolvePodKey(k) - if namespace != v[0] { + apptype, appname, podName, namespace := resolvePodKey(k) + if apptype != v[0] { + t.Fatal(apptype) + } + if namespace != v[1] { t.Fatal(namespace) } - if appname != v[1] { + if appname != v[2] { t.Fatal(appname) } - if podName != v[2] { + if podName != v[3] { t.Fatal(podName) } } @@ -78,7 +82,7 @@ func TestFormatKey(t *testing.T) { PoolName: "pl1", }, expectPoolPrefix: "pool__pl1_", - expectPoolAppPrefix: "pool__pl1_sts_ns1_sts", + expectPoolAppPrefix: "pool__pl1_sts_ns1_sts_", }, { pod: CreateDeploymentPod("dp-xxx-yyy", "ns1", nil), @@ -130,7 +134,33 @@ func TestFormatKey(t *testing.T) { PoolName: "pl1", }, expectPoolPrefix: "pool__pl1_", - expectPoolAppPrefix: "pool__pl1_tapp_ns1_tapp", + expectPoolAppPrefix: "pool__pl1_tapp_ns1_tapp_", + }, + { + pod: CreateSimplePod("pod1", "ns1", nil), + expect: KeyObj{ + KeyInDB: "NULL_ns1_NULL_pod1", + AppTypePrefix: "NULL_", + AppName: "NULL", + PodName: "pod1", + Namespace: "ns1", + PoolName: "", + }, + expectPoolPrefix: "NULL_ns1_NULL_", + expectPoolAppPrefix: "NULL_ns1_NULL_", + }, + { + pod: CreatePodWithKind("worker-1", "ns1", "spark", nil), + expect: KeyObj{ + KeyInDB: "spark_ns1_worker_worker-1", + AppTypePrefix: "spark_", + AppName: "worker", + PodName: "worker-1", + Namespace: "ns1", + PoolName: "", + }, + expectPoolPrefix: "spark_ns1_worker_", + expectPoolAppPrefix: "spark_ns1_worker_", }, } for i := range testCases { @@ -145,6 +175,9 @@ func TestFormatKey(t *testing.T) { if testCase.expectPoolPrefix != got.PoolPrefix() { t.Errorf("case %d, expect %+v, got %+v", i, testCase.expectPoolPrefix, got.PoolPrefix()) } + if testCase.expectPoolAppPrefix != got.PoolAppPrefix() { + t.Errorf("case %d, expect %+v, got %+v", i, testCase.expectPoolAppPrefix, got.PoolAppPrefix()) + } } } @@ -310,6 +343,19 @@ func TestParseKey(t *testing.T) { }, expectPoolPrefix: "tapp_ns1_demo_", }, + // unknown app type + { + keyInDB: "spark_ns1_worker_worker-1", + expect: KeyObj{ + KeyInDB: "spark_ns1_worker_worker-1", + AppTypePrefix: "spark_", + AppName: "worker", + PodName: "worker-1", + Namespace: "ns1", + PoolName: "", + }, + expectPoolPrefix: "spark_ns1_worker_", + }, } for i := range testCases { testCase := testCases[i]