Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(kubernetes): Clean up KubernetesCacheUtils class (#4764)
* refactor(kubernetes): Extract getHandler to a function This is a minor refactor to make the upcoming changes cleaner; just pull out a function. * refactor(kubernetes): Fix type safety and null safety resourceModelFromCacheData is very unsafe; it does an unchecked cast and can also return null. The goal of that function is to allow custom overrides for how to convert cache data into a ServerGroup or a ServerGroupManager. (We have not implemented the ability to customize other types.) The problem is that we are checking if the handler is of the generic ModelHandler instead of the specific type we are interested in. We already have ServerGroupHandler and ServerGroupManagerHandler to handle these two specific cases. Split the generic resourceModelFromCacheData into to specific ones to handle these two cases. By checking if the handler implements the specific case, we can avoid unchecked casts. Finally, rather than return null if the handler doesn't specify an override, just use the default implementation. The goal here is only to allow people to override this if they want; if they don't override, we can just use the default. * refactor(kubernetes): Clean up CustomKubernetesHandlerFactory This class implemented the (raw) ModelHandler, but only ever implemented the default implementations for the kind in question. As we now fall back to the default implementations in cases where there is no override, we don't need to explicitly override them here. * refactor(kubernetes): Remove ModelHandler class Now that we're using the more specific handler classes to detect custom implementations of fromCacheData, we don't need the ModelHandler interface at all anymore; remove it. * refactor(kubernetes): Add relationshipTypes helper function Minor refactor to abstract away translating a Spinnaker kind into its relationship types. * refactor(kubernetes): Add relationshipKeys function Minor refactor to pull common logic looking up relationships on a CacheData object into a function. * refactor(kubernetes): Rename to function Rename aggregateRelationshipsBySpinnakerKind as getRelationshipKeys, because it does the same thing as the relationshipKeys function we just created, but for a SpinnakerKind. Also collect to an ImmutableList and update the return type. Similarly remove the OfSpinnakerKind from getRelationships, becase the function signature implies what the input is. * refactor(kubernetes): Update return type of mapByRelationship It will now return a Map<String, Collection<CacheData>> instead of a Map<String, List<CacheData>> because it's not actually important to callers that the values are List<?>. This will make the next commit possible. * refactor(kubernetes): mapByRelationship returns a MultiMap This is a much better data structure for mapping keys to multiple values. In particular, it has the property that .get(key) returns an empty list rather than null if there are no mappings, so we can avoid a lot of defaulting. * refactor(kubernetes): Inline a variable Inline the keys variable in loadRelationshipsFromCache; also collect the relationship keys to a Set rather than to a List. This cuts down the number of times we look up load balancers from 4 to 2; we'll still need to get that down to 1 but this is progress. * refactor(kubernetes): Rename getTransitiveRelationships Rename this function to something shorter and clearer; also update it to only accept a single CacheData as all callers are wrapping a single CacheData in a collection when calling it. * refactor(kubernetes): Require callers of to create the filter Simple refactor to push requiring the cache filter to be created to callers. (Actually caller as this is only used in one place.) * refactor(kubernetes): Require callers of getRelationships to pass data getRelationships is inherently ineffecient, as it encourages callers to require a lookup of data they already have. In 2 of the 3 callers we are either looking up the same data in a loop, or already have the data we're looking up. Require callers to first look up the cache data, then pass it in to get its relationships. * refactor(kubernetes): Rename loadRelationshipsFromCache Use getRelationships to be consistent with how we're naming other functions in this class. Also update the single-data-consuming version to call the list version with a singleton. * refactor(kubernetes): Remove loadRelationshipsFromCache This is identical to getRelationships. Also the whole class is non-null now, so move the annotation to the class level. * refactor(kubernetes): Cleanup of KubernetesCacheUtils Make the class package-private as it is only used in the package. Also add Javadoc to some methods. * refactor(kubernetes): Replace kindMap lookups with getRelationships We already have a function in cacheUtils that looks up all relationships for a SpinnakerKind; replace cases where callers map the SpinnakerKind to KubernetesKinds with just calling this function. * style(kubernetes): Fix comments Co-Authored-By: Maggie Neterval <mneterval@google.com> Co-authored-by: Maggie Neterval <mneterval@google.com>
- Loading branch information
1 parent
43a28f6
commit 3d90bde