Update CanAdmitPodResult() in TopologyManager to take a TopologyHint
Previously it only took a bool, which limited the logic it could perform to determine if a pod should be admitted or not based on the merged hint from the policy.
This commit is contained in:
		@@ -25,5 +25,5 @@ type Policy interface {
 | 
				
			|||||||
	//Returns Policy Name
 | 
						//Returns Policy Name
 | 
				
			||||||
	Name() string
 | 
						Name() string
 | 
				
			||||||
	//Returns Pod Admit Handler Response based on hints and policy type
 | 
						//Returns Pod Admit Handler Response based on hints and policy type
 | 
				
			||||||
	CanAdmitPodResult(admit bool) lifecycle.PodAdmitResult
 | 
						CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -36,7 +36,7 @@ func (p *bestEffortPolicy) Name() string {
 | 
				
			|||||||
	return PolicyBestEffort
 | 
						return PolicyBestEffort
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (p *bestEffortPolicy) CanAdmitPodResult(admit bool) lifecycle.PodAdmitResult {
 | 
					func (p *bestEffortPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult {
 | 
				
			||||||
	return lifecycle.PodAdmitResult{
 | 
						return lifecycle.PodAdmitResult{
 | 
				
			||||||
		Admit: true,
 | 
							Admit: true,
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -23,25 +23,24 @@ import (
 | 
				
			|||||||
func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
 | 
					func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
 | 
				
			||||||
	tcases := []struct {
 | 
						tcases := []struct {
 | 
				
			||||||
		name     string
 | 
							name     string
 | 
				
			||||||
		admit    bool
 | 
							hint     TopologyHint
 | 
				
			||||||
		expected bool
 | 
							expected bool
 | 
				
			||||||
	}{
 | 
						}{
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to false in topology hints",
 | 
								name:     "Preferred is set to false in topology hints",
 | 
				
			||||||
			admit:    false,
 | 
								hint:     TopologyHint{nil, false},
 | 
				
			||||||
			expected: true,
 | 
								expected: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to true in topology hints",
 | 
								name:     "Preferred is set to true in topology hints",
 | 
				
			||||||
			admit:    true,
 | 
								hint:     TopologyHint{nil, true},
 | 
				
			||||||
			expected: true,
 | 
								expected: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	for _, tc := range tcases {
 | 
						for _, tc := range tcases {
 | 
				
			||||||
		policy := NewBestEffortPolicy()
 | 
							policy := NewBestEffortPolicy()
 | 
				
			||||||
		admit := tc.admit
 | 
							result := policy.CanAdmitPodResult(&tc.hint)
 | 
				
			||||||
		result := policy.CanAdmitPodResult(admit)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if result.Admit != tc.expected {
 | 
							if result.Admit != tc.expected {
 | 
				
			||||||
			t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
								t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -36,7 +36,7 @@ func (p *nonePolicy) Name() string {
 | 
				
			|||||||
	return PolicyNone
 | 
						return PolicyNone
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (p *nonePolicy) CanAdmitPodResult(admit bool) lifecycle.PodAdmitResult {
 | 
					func (p *nonePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult {
 | 
				
			||||||
	return lifecycle.PodAdmitResult{
 | 
						return lifecycle.PodAdmitResult{
 | 
				
			||||||
		Admit: true,
 | 
							Admit: true,
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -41,25 +41,24 @@ func TestName(t *testing.T) {
 | 
				
			|||||||
func TestPolicyNoneCanAdmitPodResult(t *testing.T) {
 | 
					func TestPolicyNoneCanAdmitPodResult(t *testing.T) {
 | 
				
			||||||
	tcases := []struct {
 | 
						tcases := []struct {
 | 
				
			||||||
		name     string
 | 
							name     string
 | 
				
			||||||
		admit    bool
 | 
							hint TopologyHint
 | 
				
			||||||
		expected bool
 | 
							expected bool
 | 
				
			||||||
	}{
 | 
						}{
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to false in topology hints",
 | 
								name:     "Preferred is set to false in topology hints",
 | 
				
			||||||
			admit:    false,
 | 
								hint:     TopologyHint{nil, false},
 | 
				
			||||||
			expected: true,
 | 
								expected: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to true in topology hints",
 | 
								name:     "Preferred is set to true in topology hints",
 | 
				
			||||||
			admit:    true,
 | 
								hint:     TopologyHint{nil, true},
 | 
				
			||||||
			expected: true,
 | 
								expected: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	for _, tc := range tcases {
 | 
						for _, tc := range tcases {
 | 
				
			||||||
		policy := NewNonePolicy()
 | 
							policy := NewNonePolicy()
 | 
				
			||||||
		admit := tc.admit
 | 
							result := policy.CanAdmitPodResult(&tc.hint)
 | 
				
			||||||
		result := policy.CanAdmitPodResult(admit)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if result.Admit != tc.expected {
 | 
							if result.Admit != tc.expected {
 | 
				
			||||||
			t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
								t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -36,8 +36,8 @@ func (p *restrictedPolicy) Name() string {
 | 
				
			|||||||
	return PolicyRestricted
 | 
						return PolicyRestricted
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (p *restrictedPolicy) CanAdmitPodResult(admit bool) lifecycle.PodAdmitResult {
 | 
					func (p *restrictedPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult {
 | 
				
			||||||
	if !admit {
 | 
						if !hint.Preferred {
 | 
				
			||||||
		return lifecycle.PodAdmitResult{
 | 
							return lifecycle.PodAdmitResult{
 | 
				
			||||||
			Admit:   false,
 | 
								Admit:   false,
 | 
				
			||||||
			Reason:  "Topology Affinity Error",
 | 
								Reason:  "Topology Affinity Error",
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -23,25 +23,24 @@ import (
 | 
				
			|||||||
func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 | 
					func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 | 
				
			||||||
	tcases := []struct {
 | 
						tcases := []struct {
 | 
				
			||||||
		name     string
 | 
							name     string
 | 
				
			||||||
		admit    bool
 | 
							hint     TopologyHint
 | 
				
			||||||
		expected bool
 | 
							expected bool
 | 
				
			||||||
	}{
 | 
						}{
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to false in topology hints",
 | 
								name:     "Preferred is set to false in topology hints",
 | 
				
			||||||
			admit:    false,
 | 
								hint:     TopologyHint{nil, false},
 | 
				
			||||||
			expected: false,
 | 
								expected: false,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			name:     "Affinity is set to true in topology hints",
 | 
								name:     "Preferred is set to true in topology hints",
 | 
				
			||||||
			admit:    true,
 | 
								hint:     TopologyHint{nil, true},
 | 
				
			||||||
			expected: true,
 | 
								expected: true,
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	for _, tc := range tcases {
 | 
						for _, tc := range tcases {
 | 
				
			||||||
		policy := NewRestrictedPolicy()
 | 
							policy := NewRestrictedPolicy()
 | 
				
			||||||
		admit := tc.admit
 | 
							result := policy.CanAdmitPodResult(&tc.hint)
 | 
				
			||||||
		result := policy.CanAdmitPodResult(admit)
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
		if result.Admit != tc.expected {
 | 
							if result.Admit != tc.expected {
 | 
				
			||||||
			t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
								t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -308,7 +308,7 @@ func (m *manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitR
 | 
				
			|||||||
	if pod.Status.QOSClass == v1.PodQOSGuaranteed {
 | 
						if pod.Status.QOSClass == v1.PodQOSGuaranteed {
 | 
				
			||||||
		for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) {
 | 
							for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) {
 | 
				
			||||||
			result := m.calculateAffinity(*pod, container)
 | 
								result := m.calculateAffinity(*pod, container)
 | 
				
			||||||
			admitPod := m.policy.CanAdmitPodResult(result.Preferred)
 | 
								admitPod := m.policy.CanAdmitPodResult(&result)
 | 
				
			||||||
			if !admitPod.Admit {
 | 
								if !admitPod.Admit {
 | 
				
			||||||
				return admitPod
 | 
									return admitPod
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user