Make registered more testable

pkg/apimachinery/registered/registered.go

@@ -32,8 +32,13 @@ import (
 )
 
 var (
+	DefaultAPIRegistrationManager = NewOrDie(os.Getenv("KUBE_API_VERSIONS"))
+)
+
+// APIRegistrationManager
+type APIRegistrationManager struct {
 	// registeredGroupVersions stores all API group versions for which RegisterGroup is called.
-	registeredVersions = map[unversioned.GroupVersion]struct{}{}
+	registeredVersions map[unversioned.GroupVersion]struct{}
 
 	// thirdPartyGroupVersions are API versions which are dynamically
 	// registered (and unregistered) via API calls to the apiserver
@@ -42,75 +47,99 @@ var (
 	// enabledVersions represents all enabled API versions. It should be a
 	// subset of registeredVersions. Please call EnableVersions() to add
 	// enabled versions.
-	enabledVersions = map[unversioned.GroupVersion]struct{}{}
+	enabledVersions map[unversioned.GroupVersion]struct{}
 
 	// map of group meta for all groups.
-	groupMetaMap = map[string]*apimachinery.GroupMeta{}
+	groupMetaMap map[string]*apimachinery.GroupMeta
 
 	// envRequestedVersions represents the versions requested via the
 	// KUBE_API_VERSIONS environment variable. The install package of each group
 	// checks this list before add their versions to the latest package and
 	// Scheme.  This list is small and order matters, so represent as a slice
-	envRequestedVersions = []unversioned.GroupVersion{}
+	envRequestedVersions []unversioned.GroupVersion
-)
-
-func init() {
-	loadKubeAPIVersions()
 }
 
-func loadKubeAPIVersions() {
+// NewAPIRegistrationManager constructs a new manager. The argument ought to be
-	// Env var KUBE_API_VERSIONS is a comma separated list of API versions that
+// the value of the KUBE_API_VERSIONS env var, or a value of this which you
-	// should be registered in the scheme.
+// wish to test.
-	kubeAPIVersions := os.Getenv("KUBE_API_VERSIONS")
+func NewAPIRegistrationManager(kubeAPIVersions string) (*APIRegistrationManager, error) {
+	m := &APIRegistrationManager{
+		registeredVersions:      map[unversioned.GroupVersion]struct{}{},
+		thirdPartyGroupVersions: []unversioned.GroupVersion{},
+		enabledVersions:         map[unversioned.GroupVersion]struct{}{},
+		groupMetaMap:            map[string]*apimachinery.GroupMeta{},
+		envRequestedVersions:    []unversioned.GroupVersion{},
+	}
+
 	if len(kubeAPIVersions) != 0 {
 		for _, version := range strings.Split(kubeAPIVersions, ",") {
 			gv, err := unversioned.ParseGroupVersion(version)
 			if err != nil {
-				glog.Fatalf("invalid api version: %s in KUBE_API_VERSIONS: %s.",
+				return nil, fmt.Errorf("invalid api version: %s in KUBE_API_VERSIONS: %s.",
-					version, os.Getenv("KUBE_API_VERSIONS"))
+					version, kubeAPIVersions)
 			}
-			envRequestedVersions = append(envRequestedVersions, gv)
+			m.envRequestedVersions = append(m.envRequestedVersions, gv)
 		}
 	}
+	return m, nil
 }
 
-// Resets everything to clean room for the start of a test
+func NewOrDie(kubeAPIVersions string) *APIRegistrationManager {
-func clearForTesting() {
+	m, err := NewAPIRegistrationManager(kubeAPIVersions)
-	registeredVersions = map[unversioned.GroupVersion]struct{}{}
+	if err != nil {
-	thirdPartyGroupVersions = []unversioned.GroupVersion{}
+		glog.Fatalf("Could not construct version manager: %v (KUBE_API_VERSIONS=%q)", err, kubeAPIVersions)
-	enabledVersions = map[unversioned.GroupVersion]struct{}{}
+	}
-	groupMetaMap = map[string]*apimachinery.GroupMeta{}
+	return m
-	envRequestedVersions = []unversioned.GroupVersion{}
-	loadKubeAPIVersions()
 }
 
+// People are calling global functions. Let them continue to do that (for now).
+var (
+	ValidateEnvRequestedVersions  = DefaultAPIRegistrationManager.ValidateEnvRequestedVersions
+	AllPreferredGroupVersions     = DefaultAPIRegistrationManager.AllPreferredGroupVersions
+	RESTMapper                    = DefaultAPIRegistrationManager.RESTMapper
+	GroupOrDie                    = DefaultAPIRegistrationManager.GroupOrDie
+	AddThirdPartyAPIGroupVersions = DefaultAPIRegistrationManager.AddThirdPartyAPIGroupVersions
+	IsThirdPartyAPIGroupVersion   = DefaultAPIRegistrationManager.IsThirdPartyAPIGroupVersion
+	RegisteredGroupVersions       = DefaultAPIRegistrationManager.RegisteredGroupVersions
+	IsRegisteredVersion           = DefaultAPIRegistrationManager.IsRegisteredVersion
+	IsRegistered                  = DefaultAPIRegistrationManager.IsRegistered
+	Group                         = DefaultAPIRegistrationManager.Group
+	EnabledVersionsForGroup       = DefaultAPIRegistrationManager.EnabledVersionsForGroup
+	EnabledVersions               = DefaultAPIRegistrationManager.EnabledVersions
+	IsEnabledVersion              = DefaultAPIRegistrationManager.IsEnabledVersion
+	IsAllowedVersion              = DefaultAPIRegistrationManager.IsAllowedVersion
+	EnableVersions                = DefaultAPIRegistrationManager.EnableVersions
+	RegisterGroup                 = DefaultAPIRegistrationManager.RegisterGroup
+	RegisterVersions              = DefaultAPIRegistrationManager.RegisterVersions
+)
+
 // RegisterVersions adds the given group versions to the list of registered group versions.
-func RegisterVersions(availableVersions []unversioned.GroupVersion) {
+func (m *APIRegistrationManager) RegisterVersions(availableVersions []unversioned.GroupVersion) {
 	for _, v := range availableVersions {
-		registeredVersions[v] = struct{}{}
+		m.registeredVersions[v] = struct{}{}
 	}
 }
 
 // RegisterGroup adds the given group to the list of registered groups.
-func RegisterGroup(groupMeta apimachinery.GroupMeta) error {
+func (m *APIRegistrationManager) RegisterGroup(groupMeta apimachinery.GroupMeta) error {
 	groupName := groupMeta.GroupVersion.Group
-	if _, found := groupMetaMap[groupName]; found {
+	if _, found := m.groupMetaMap[groupName]; found {
-		return fmt.Errorf("group %v is already registered", groupMetaMap)
+		return fmt.Errorf("group %v is already registered", m.groupMetaMap)
 	}
-	groupMetaMap[groupName] = &groupMeta
+	m.groupMetaMap[groupName] = &groupMeta
 	return nil
 }
 
 // EnableVersions adds the versions for the given group to the list of enabled versions.
 // Note that the caller should call RegisterGroup before calling this method.
 // The caller of this function is responsible to add the versions to scheme and RESTMapper.
-func EnableVersions(versions ...unversioned.GroupVersion) error {
+func (m *APIRegistrationManager) EnableVersions(versions ...unversioned.GroupVersion) error {
 	var unregisteredVersions []unversioned.GroupVersion
 	for _, v := range versions {
-		if _, found := registeredVersions[v]; !found {
+		if _, found := m.registeredVersions[v]; !found {
 			unregisteredVersions = append(unregisteredVersions, v)
 		}
-		enabledVersions[v] = struct{}{}
+		m.enabledVersions[v] = struct{}{}
 	}
 	if len(unregisteredVersions) != 0 {
 		return fmt.Errorf("Please register versions before enabling them: %v", unregisteredVersions)
@@ -121,11 +150,11 @@ func EnableVersions(versions ...unversioned.GroupVersion) error {
 // IsAllowedVersion returns if the version is allowed by the KUBE_API_VERSIONS
 // environment variable. If the environment variable is empty, then it always
 // returns true.
-func IsAllowedVersion(v unversioned.GroupVersion) bool {
+func (m *APIRegistrationManager) IsAllowedVersion(v unversioned.GroupVersion) bool {
-	if len(envRequestedVersions) == 0 {
+	if len(m.envRequestedVersions) == 0 {
 		return true
 	}
-	for _, envGV := range envRequestedVersions {
+	for _, envGV := range m.envRequestedVersions {
 		if v == envGV {
 			return true
 		}
@@ -134,24 +163,24 @@ func IsAllowedVersion(v unversioned.GroupVersion) bool {
 }
 
 // IsEnabledVersion returns if a version is enabled.
-func IsEnabledVersion(v unversioned.GroupVersion) bool {
+func (m *APIRegistrationManager) IsEnabledVersion(v unversioned.GroupVersion) bool {
-	_, found := enabledVersions[v]
+	_, found := m.enabledVersions[v]
 	return found
 }
 
 // EnabledVersions returns all enabled versions.  Groups are randomly ordered, but versions within groups
 // are priority order from best to worst
-func EnabledVersions() []unversioned.GroupVersion {
+func (m *APIRegistrationManager) EnabledVersions() []unversioned.GroupVersion {
 	ret := []unversioned.GroupVersion{}
-	for _, groupMeta := range groupMetaMap {
+	for _, groupMeta := range m.groupMetaMap {
 		ret = append(ret, groupMeta.GroupVersions...)
 	}
 	return ret
 }
 
 // EnabledVersionsForGroup returns all enabled versions for a group in order of best to worst
-func EnabledVersionsForGroup(group string) []unversioned.GroupVersion {
+func (m *APIRegistrationManager) EnabledVersionsForGroup(group string) []unversioned.GroupVersion {
-	groupMeta, ok := groupMetaMap[group]
+	groupMeta, ok := m.groupMetaMap[group]
 	if !ok {
 		return []unversioned.GroupVersion{}
 	}
@@ -159,10 +188,10 @@ func EnabledVersionsForGroup(group string) []unversioned.GroupVersion {
 	return append([]unversioned.GroupVersion{}, groupMeta.GroupVersions...)
 }
 
-// Group returns the metadata of a group if the gruop is registered, otherwise
+// Group returns the metadata of a group if the group is registered, otherwise
-// an erorr is returned.
+// an error is returned.
-func Group(group string) (*apimachinery.GroupMeta, error) {
+func (m *APIRegistrationManager) Group(group string) (*apimachinery.GroupMeta, error) {
-	groupMeta, found := groupMetaMap[group]
+	groupMeta, found := m.groupMetaMap[group]
 	if !found {
 		return nil, fmt.Errorf("group %v has not been registered", group)
 	}
@@ -171,30 +200,30 @@ func Group(group string) (*apimachinery.GroupMeta, error) {
 }
 
 // IsRegistered takes a string and determines if it's one of the registered groups
-func IsRegistered(group string) bool {
+func (m *APIRegistrationManager) IsRegistered(group string) bool {
-	_, found := groupMetaMap[group]
+	_, found := m.groupMetaMap[group]
 	return found
 }
 
 // IsRegisteredVersion returns if a version is registered.
-func IsRegisteredVersion(v unversioned.GroupVersion) bool {
+func (m *APIRegistrationManager) IsRegisteredVersion(v unversioned.GroupVersion) bool {
-	_, found := registeredVersions[v]
+	_, found := m.registeredVersions[v]
 	return found
 }
 
 // RegisteredGroupVersions returns all registered group versions.
-func RegisteredGroupVersions() []unversioned.GroupVersion {
+func (m *APIRegistrationManager) RegisteredGroupVersions() []unversioned.GroupVersion {
 	ret := []unversioned.GroupVersion{}
-	for groupVersion := range registeredVersions {
+	for groupVersion := range m.registeredVersions {
 		ret = append(ret, groupVersion)
 	}
 	return ret
 }
 
 // IsThirdPartyAPIGroupVersion returns true if the api version is a user-registered group/version.
-func IsThirdPartyAPIGroupVersion(gv unversioned.GroupVersion) bool {
+func (m *APIRegistrationManager) IsThirdPartyAPIGroupVersion(gv unversioned.GroupVersion) bool {
-	for ix := range thirdPartyGroupVersions {
+	for ix := range m.thirdPartyGroupVersions {
-		if thirdPartyGroupVersions[ix] == gv {
+		if m.thirdPartyGroupVersions[ix] == gv {
 			return true
 		}
 	}
@@ -205,11 +234,11 @@ func IsThirdPartyAPIGroupVersion(gv unversioned.GroupVersion) bool {
 // registers them in the API machinery and enables them.
 // Skips GroupVersions that are already registered.
 // Returns the list of GroupVersions that were skipped.
-func AddThirdPartyAPIGroupVersions(gvs ...unversioned.GroupVersion) []unversioned.GroupVersion {
+func (m *APIRegistrationManager) AddThirdPartyAPIGroupVersions(gvs ...unversioned.GroupVersion) []unversioned.GroupVersion {
 	filteredGVs := []unversioned.GroupVersion{}
 	skippedGVs := []unversioned.GroupVersion{}
 	for ix := range gvs {
-		if !IsRegisteredVersion(gvs[ix]) {
+		if !m.IsRegisteredVersion(gvs[ix]) {
 			filteredGVs = append(filteredGVs, gvs[ix])
 		} else {
 			glog.V(3).Infof("Skipping %s, because its already registered", gvs[ix].String())
@@ -219,17 +248,18 @@ func AddThirdPartyAPIGroupVersions(gvs ...unversioned.GroupVersion) []unversione
 	if len(filteredGVs) == 0 {
 		return skippedGVs
 	}
-	RegisterVersions(filteredGVs)
+	m.RegisterVersions(filteredGVs)
-	EnableVersions(filteredGVs...)
+	m.EnableVersions(filteredGVs...)
-	thirdPartyGroupVersions = append(thirdPartyGroupVersions, filteredGVs...)
+	m.thirdPartyGroupVersions = append(m.thirdPartyGroupVersions, filteredGVs...)
+
 	return skippedGVs
 }
 
 // TODO: This is an expedient function, because we don't check if a Group is
 // supported throughout the code base. We will abandon this function and
 // checking the error returned by the Group() function.
-func GroupOrDie(group string) *apimachinery.GroupMeta {
+func (m *APIRegistrationManager) GroupOrDie(group string) *apimachinery.GroupMeta {
-	groupMeta, found := groupMetaMap[group]
+	groupMeta, found := m.groupMetaMap[group]
 	if !found {
 		if group == "" {
 			panic("The legacy v1 API is not registered.")
@@ -246,13 +276,13 @@ func GroupOrDie(group string) *apimachinery.GroupMeta {
 //  1. legacy kube group preferred version, extensions preferred version, metrics perferred version, legacy
 //     kube any version, extensions any version, metrics any version, all other groups alphabetical preferred version,
 //     all other groups alphabetical.
-func RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper {
+func (m *APIRegistrationManager) RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper {
 	unionMapper := meta.MultiRESTMapper{}
 	unionedGroups := sets.NewString()
-	for enabledVersion := range enabledVersions {
+	for enabledVersion := range m.enabledVersions {
 		if !unionedGroups.Has(enabledVersion.Group) {
 			unionedGroups.Insert(enabledVersion.Group)
-			groupMeta := groupMetaMap[enabledVersion.Group]
+			groupMeta := m.groupMetaMap[enabledVersion.Group]
 			unionMapper = append(unionMapper, groupMeta.RESTMapper)
 		}
 	}
@@ -268,11 +298,11 @@ func RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper {
 		return meta.PriorityRESTMapper{Delegate: unionMapper, ResourcePriority: resourcePriority, KindPriority: kindPriority}
 	}
 
-	if len(envRequestedVersions) != 0 {
+	if len(m.envRequestedVersions) != 0 {
 		resourcePriority := []unversioned.GroupVersionResource{}
 		kindPriority := []unversioned.GroupVersionKind{}
 
-		for _, versionPriority := range envRequestedVersions {
+		for _, versionPriority := range m.envRequestedVersions {
 			resourcePriority = append(resourcePriority, versionPriority.WithResource(meta.AnyResource))
 			kindPriority = append(kindPriority, versionPriority.WithKind(meta.AnyKind))
 		}
@@ -281,17 +311,17 @@ func RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper {
 	}
 
 	prioritizedGroups := []string{"", "extensions", "metrics"}
-	resourcePriority, kindPriority := prioritiesForGroups(prioritizedGroups...)
+	resourcePriority, kindPriority := m.prioritiesForGroups(prioritizedGroups...)
 
 	prioritizedGroupsSet := sets.NewString(prioritizedGroups...)
 	remainingGroups := sets.String{}
-	for enabledVersion := range enabledVersions {
+	for enabledVersion := range m.enabledVersions {
 		if !prioritizedGroupsSet.Has(enabledVersion.Group) {
 			remainingGroups.Insert(enabledVersion.Group)
 		}
 	}
 
-	remainingResourcePriority, remainingKindPriority := prioritiesForGroups(remainingGroups.List()...)
+	remainingResourcePriority, remainingKindPriority := m.prioritiesForGroups(remainingGroups.List()...)
 	resourcePriority = append(resourcePriority, remainingResourcePriority...)
 	kindPriority = append(kindPriority, remainingKindPriority...)
 
@@ -300,12 +330,12 @@ func RESTMapper(versionPatterns ...unversioned.GroupVersion) meta.RESTMapper {
 
 // prioritiesForGroups returns the resource and kind priorities for a PriorityRESTMapper, preferring the preferred version of each group first,
 // then any non-preferred version of the group second.
-func prioritiesForGroups(groups ...string) ([]unversioned.GroupVersionResource, []unversioned.GroupVersionKind) {
+func (m *APIRegistrationManager) prioritiesForGroups(groups ...string) ([]unversioned.GroupVersionResource, []unversioned.GroupVersionKind) {
 	resourcePriority := []unversioned.GroupVersionResource{}
 	kindPriority := []unversioned.GroupVersionKind{}
 
 	for _, group := range groups {
-		availableVersions := EnabledVersionsForGroup(group)
+		availableVersions := m.EnabledVersionsForGroup(group)
 		if len(availableVersions) > 0 {
 			resourcePriority = append(resourcePriority, availableVersions[0].WithResource(meta.AnyResource))
 			kindPriority = append(kindPriority, availableVersions[0].WithKind(meta.AnyKind))
@@ -321,12 +351,12 @@ func prioritiesForGroups(groups ...string) ([]unversioned.GroupVersionResource,
 
 // AllPreferredGroupVersions returns the preferred versions of all registered
 // groups in the form of "group1/version1,group2/version2,..."
-func AllPreferredGroupVersions() string {
+func (m *APIRegistrationManager) AllPreferredGroupVersions() string {
-	if len(groupMetaMap) == 0 {
+	if len(m.groupMetaMap) == 0 {
 		return ""
 	}
 	var defaults []string
-	for _, groupMeta := range groupMetaMap {
+	for _, groupMeta := range m.groupMetaMap {
 		defaults = append(defaults, groupMeta.GroupVersion.String())
 	}
 	sort.Strings(defaults)
@@ -335,21 +365,12 @@ func AllPreferredGroupVersions() string {
 
 // ValidateEnvRequestedVersions returns a list of versions that are requested in
 // the KUBE_API_VERSIONS environment variable, but not enabled.
-func ValidateEnvRequestedVersions() []unversioned.GroupVersion {
+func (m *APIRegistrationManager) ValidateEnvRequestedVersions() []unversioned.GroupVersion {
 	var missingVersions []unversioned.GroupVersion
-	for _, v := range envRequestedVersions {
+	for _, v := range m.envRequestedVersions {
-		if _, found := enabledVersions[v]; !found {
+		if _, found := m.enabledVersions[v]; !found {
 			missingVersions = append(missingVersions, v)
 		}
 	}
 	return missingVersions
 }
-
-// Resets the state.
-// Should not be used by anyone else than tests.
-func reset() {
-	registeredVersions = map[unversioned.GroupVersion]struct{}{}
-	enabledVersions = map[unversioned.GroupVersion]struct{}{}
-	groupMetaMap = map[string]*apimachinery.GroupMeta{}
-
-}

pkg/apimachinery/registered/registered_test.go

@@ -24,7 +24,10 @@ import (
 )
 
 func TestAddThirdPartyVersionsBasic(t *testing.T) {
-	clearForTesting()
+	m, err := NewAPIRegistrationManager("")
+	if err != nil {
+		t.Fatalf("Unexpected failure to make a manager: %v", err)
+	}
 
 	registered := []unversioned.GroupVersion{
 		{
@@ -45,8 +48,8 @@ func TestAddThirdPartyVersionsBasic(t *testing.T) {
 	}
 	gvs := append(registered, thirdParty...)
 
-	RegisterVersions(registered)
+	m.RegisterVersions(registered)
-	wasSkipped := AddThirdPartyAPIGroupVersions(gvs...)
+	wasSkipped := m.AddThirdPartyAPIGroupVersions(gvs...)
 	if len(wasSkipped) != len(skipped) {
 		t.Errorf("Expected %v, found %v", skipped, wasSkipped)
 	}
@@ -63,15 +66,13 @@ func TestAddThirdPartyVersionsBasic(t *testing.T) {
 		}
 	}
 	for _, gv := range thirdParty {
-		if !IsThirdPartyAPIGroupVersion(gv) {
+		if !m.IsThirdPartyAPIGroupVersion(gv) {
 			t.Errorf("Expected %v to be third party.", gv)
 		}
 	}
 }
 
 func TestAddThirdPartyVersionsMultiple(t *testing.T) {
-	clearForTesting()
-
 	thirdParty := []unversioned.GroupVersion{
 		{
 			Group:   "company.com",
@@ -82,14 +83,18 @@ func TestAddThirdPartyVersionsMultiple(t *testing.T) {
 			Version: "v2",
 		},
 	}
+	m, err := NewAPIRegistrationManager("")
+	if err != nil {
+		t.Fatalf("Unexpected failure to make a manager: %v", err)
+	}
 	for _, gv := range thirdParty {
-		wasSkipped := AddThirdPartyAPIGroupVersions(gv)
+		wasSkipped := m.AddThirdPartyAPIGroupVersions(gv)
 		if len(wasSkipped) != 0 {
 			t.Errorf("Expected length 0, found %v", wasSkipped)
 		}
 	}
 	for _, gv := range thirdParty {
-		if !IsThirdPartyAPIGroupVersion(gv) {
+		if !m.IsThirdPartyAPIGroupVersion(gv) {
 			t.Errorf("Expected %v to be third party.", gv)
 		}
 	}
@@ -128,13 +133,16 @@ func TestAllPreferredGroupVersions(t *testing.T) {
 		},
 	}
 	for _, testCase := range testCases {
-		for _, groupMeta := range testCase.groupMetas {
+		m, err := NewAPIRegistrationManager("")
-			RegisterGroup(groupMeta)
+		if err != nil {
+			t.Fatalf("Unexpected failure to make a manager: %v", err)
 		}
-		output := AllPreferredGroupVersions()
+		for _, groupMeta := range testCase.groupMetas {
+			m.RegisterGroup(groupMeta)
+		}
+		output := m.AllPreferredGroupVersions()
 		if testCase.expect != output {
 			t.Errorf("Error. expect: %s, got: %s", testCase.expect, output)
 		}
-		reset()
 	}
 }