diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go index ab1f40cdc1f..488ce21330e 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go @@ -98,16 +98,20 @@ func NewLocalAvailableAPIServiceCondition() APIServiceCondition { } } +// GetAPIServiceConditionByType gets an *APIServiceCondition by APIServiceConditionType if present +func GetAPIServiceConditionByType(apiService *APIService, conditionType APIServiceConditionType) *APIServiceCondition { + for i := range apiService.Status.Conditions { + if apiService.Status.Conditions[i].Type == conditionType { + return &apiService.Status.Conditions[i] + } + } + return nil +} + // SetAPIServiceCondition sets the status condition. It either overwrites the existing one or // creates a new one func SetAPIServiceCondition(apiService *APIService, newCondition APIServiceCondition) { - var existingCondition *APIServiceCondition - for i := range apiService.Status.Conditions { - if apiService.Status.Conditions[i].Type == newCondition.Type { - existingCondition = &apiService.Status.Conditions[i] - break - } - } + existingCondition := GetAPIServiceConditionByType(apiService, newCondition.Type) if existingCondition == nil { apiService.Status.Conditions = append(apiService.Status.Conditions, newCondition) return @@ -124,10 +128,6 @@ func SetAPIServiceCondition(apiService *APIService, newCondition APIServiceCondi // IsAPIServiceConditionTrue indicates if the condition is present and strictly true func IsAPIServiceConditionTrue(apiService *APIService, conditionType APIServiceConditionType) bool { - for _, condition := range apiService.Status.Conditions { - if condition.Type == conditionType && condition.Status == ConditionTrue { - return true - } - } - return false + condition := GetAPIServiceConditionByType(apiService, conditionType) + return condition != nil && condition.Status == ConditionTrue } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers_test.go index 7bca228bdc0..59b0ed61dee 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers_test.go @@ -21,6 +21,124 @@ import ( "testing" ) +var ( + a APIServiceConditionType = "A" + b APIServiceConditionType = "B" + c APIServiceConditionType = "C" +) + +func TestGetAPIServiceConditionByType(t *testing.T) { + conditionA := makeNewAPIServiceCondition(a, "a reason", "a message", ConditionTrue) + conditionB := makeNewAPIServiceCondition(b, "b reason", "b message", ConditionTrue) + tests := []*struct { + name string + apiService *APIService + conditionType APIServiceConditionType + expectedCondition *APIServiceCondition + }{ + { + name: "Should find a matching condition from apiService", + apiService: makeNewApiService("v1", 100, conditionA, conditionB), + conditionType: a, + expectedCondition: &conditionA, + }, + { + name: "Should not find a matching condition", + apiService: makeNewApiService("v1", 100, conditionA), + conditionType: b, + expectedCondition: nil, + }, + } + + for _, tc := range tests { + actual := GetAPIServiceConditionByType(tc.apiService, tc.conditionType) + if !reflect.DeepEqual(tc.expectedCondition, actual) { + t.Errorf("expected %s, actual %s", tc.expectedCondition, actual) + } + } +} + +func TestIsAPIServiceConditionTrue(t *testing.T) { + conditionATrue := makeNewAPIServiceCondition(a, "a reason", "a message", ConditionTrue) + conditionAFalse := makeNewAPIServiceCondition(a, "a reason", "a message", ConditionFalse) + tests := []*struct { + name string + apiService *APIService + conditionType APIServiceConditionType + expected bool + }{ + { + name: "Should return false when condition of type is not present", + apiService: makeNewApiService("v1", 100), + conditionType: a, + expected: false, + }, + { + name: "Should return false when condition of type is present but status is not ConditionTrue", + apiService: makeNewApiService("v1", 100, conditionAFalse), + conditionType: a, + expected: false, + }, + { + name: "Should return false when condition of type is present but status is not ConditionTrue", + apiService: makeNewApiService("v1", 100, conditionATrue), + conditionType: a, + expected: true, + }, + } + + for _, tc := range tests { + if isConditionTrue := IsAPIServiceConditionTrue(tc.apiService, tc.conditionType); isConditionTrue != tc.expected { + t.Errorf("expected condition of type %v to be %v, actually was %v", + tc.conditionType, isConditionTrue, tc.expected) + + } + } +} + +func TestSetAPIServiceCondition(t *testing.T) { + conditionA1 := makeNewAPIServiceCondition(a, "a1 reason", "a1 message", ConditionTrue) + conditionA2 := makeNewAPIServiceCondition(a, "a2 reason", "a2 message", ConditionTrue) + tests := []*struct { + name string + apiService *APIService + conditionType APIServiceConditionType + initialCondition *APIServiceCondition + setCondition APIServiceCondition + expectedCondition *APIServiceCondition + }{ + { + name: "Should set a new condition with type where previously there was no condition of that type", + apiService: makeNewApiService("v1", 100), + conditionType: a, + initialCondition: nil, + setCondition: conditionA1, + expectedCondition: &conditionA1, + }, + { + name: "Should override a condition of type, when a condition of that type existed previously", + apiService: makeNewApiService("v1", 100, conditionA1), + conditionType: a, + initialCondition: &conditionA1, + setCondition: conditionA2, + expectedCondition: &conditionA2, + }, + } + + for _, tc := range tests { + startingCondition := GetAPIServiceConditionByType(tc.apiService, tc.conditionType) + if !reflect.DeepEqual(startingCondition, tc.initialCondition) { + t.Errorf("expected to find condition %s initially, actual was %s", tc.initialCondition, startingCondition) + + } + SetAPIServiceCondition(tc.apiService, tc.setCondition) + actual := GetAPIServiceConditionByType(tc.apiService, tc.setCondition.Type) + if !reflect.DeepEqual(actual, tc.expectedCondition) { + t.Errorf("expected %s, actual %s", tc.expectedCondition, actual) + } + } +} + func TestSortedAPIServicesByVersion(t *testing.T) { tests := []*struct { name string @@ -55,12 +173,12 @@ func TestSortedAPIServicesByVersion(t *testing.T) { } for _, tc := range tests { - apiServices := []*APIService{} + apiServices := make([]*APIService, 0) for _, v := range tc.versions { - apiServices = append(apiServices, &APIService{Spec: APIServiceSpec{Version: v, VersionPriority: 100}}) + apiServices = append(apiServices, makeNewApiService(v, 100)) } sortedServices := SortedByGroupAndVersion(apiServices) - actual := []string{} + actual := make([]string, 0) for _, s := range sortedServices[0] { actual = append(actual, s.Spec.Version) } @@ -69,3 +187,12 @@ func TestSortedAPIServicesByVersion(t *testing.T) { } } } + +func makeNewApiService(version string, priority int32, conditions ...APIServiceCondition) *APIService { + status := APIServiceStatus{Conditions: conditions} + return &APIService{Spec: APIServiceSpec{Version: version, VersionPriority: priority}, Status: status} +} + +func makeNewAPIServiceCondition(conditionType APIServiceConditionType, reason string, message string, status ConditionStatus) APIServiceCondition { + return APIServiceCondition{Type: conditionType, Reason: reason, Message: message, Status: status} +}