Skip to content

Commit

Permalink
Review Comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Feny Mehta <[email protected]>
  • Loading branch information
@fbm3307
fbm3307 committed Mar 19, 2024
1 parent 61beb0c commit ff9924f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 63 deletions.
30 changes: 6 additions & 24 deletions pkg/cluster/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ func GetCachedToolchainCluster(name string) (*CachedToolchainCluster, bool) {
return clusterCache.getCachedToolchainCluster(name, true)
}

// GetClustersFunc a func that returns all the clusters from the cache
type GetClustersFunc func(conditions ...Condition) []*CachedToolchainCluster
// GetToolchainClustersCachedFunc a func that returns all the clusters from the cache
type GetToolchainClustersCachedFunc func(conditions ...Condition) []*CachedToolchainCluster

// Clusters the func to retrieve all the clusters
var Clusters GetClustersFunc = GetClusters
// ToolchainClustersCached the func to retrieve all the clusters
var ToolchainClustersCached GetToolchainClustersCachedFunc = GetToolchainClustersCached

// GetClusters returns the kube clients for all the clusters from the cache of the clusters
func GetClusters(conditions ...Condition) []*CachedToolchainCluster {
// GetToolchainClustersCached returns the kube clients for all the clusters from the cache of the clusters
func GetToolchainClustersCached(conditions ...Condition) []*CachedToolchainCluster {
clusters := clusterCache.getCachedToolchainClusters(conditions...)
if len(clusters) == 0 {
if clusterCache.refreshCache != nil {
Expand Down Expand Up @@ -144,24 +144,6 @@ func GetHostCluster() (*CachedToolchainCluster, bool) {
return clusters[0], true
}

// GetMemberClustersFunc a func that returns the member clusters from the cache
type GetMemberClustersFunc func(conditions ...Condition) []*CachedToolchainCluster

// MemberClusters the func to retrieve the member clusters
var MemberClusters GetMemberClustersFunc = GetMemberClusters

// GetMemberClusters returns the kube clients for the host clusters from the cache of the clusters
func GetMemberClusters(conditions ...Condition) []*CachedToolchainCluster {
clusters := clusterCache.getCachedToolchainClusters(conditions...)
if len(clusters) == 0 {
if clusterCache.refreshCache != nil {
clusterCache.refreshCache()
}
clusters = clusterCache.getCachedToolchainClusters(conditions...)
}
return clusters
}

// Role defines the role of the cluster.
// Each type of cluster can have multiple roles (tenant for specific APIs, user workloads, others ... )
type Role string
Expand Down
8 changes: 4 additions & 4 deletions pkg/cluster/cache_whitebox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestGetClusters(t *testing.T) {
// no clusters

//when
clusters := GetClusters()
clusters := GetToolchainClustersCached()

//then
assert.Empty(t, clusters)
Expand All @@ -92,7 +92,7 @@ func TestGetClusters(t *testing.T) {
clusterCache.addCachedToolchainCluster(member2)

//when
clusters := GetClusters()
clusters := GetToolchainClustersCached()

//then
assert.Len(t, clusters, 3)
Expand All @@ -112,7 +112,7 @@ func TestGetClusters(t *testing.T) {
}

//when
clusters := GetClusters()
clusters := GetToolchainClustersCached()

//then
assert.Len(t, clusters, 1)
Expand All @@ -134,7 +134,7 @@ func TestGetClusters(t *testing.T) {
clusterCache.addCachedToolchainCluster(member3)

//when
clusters := GetClusters(Ready)
clusters := GetToolchainClustersCached(Ready)

//then
assert.Len(t, clusters, 3)
Expand Down
11 changes: 5 additions & 6 deletions pkg/status/toolchaincluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,19 @@ const (

// ToolchainClusterAttributes required attributes for obtaining ToolchainCluster status
type ToolchainClusterAttributes struct {
GetClustersFunc cluster.GetClustersFunc
Period time.Duration
Timeout time.Duration
GetClusterFunc func() (*cluster.CachedToolchainCluster, bool)
Period time.Duration
Timeout time.Duration
}

// GetToolchainClusterConditions uses the provided ToolchainCluster attributes to determine status conditions
func GetToolchainClusterConditions(logger logr.Logger, attrs ToolchainClusterAttributes) []toolchainv1alpha1.Condition {
// look up cluster connection status
toolchainClusters := attrs.GetClustersFunc()
toolchainCluster, ok := attrs.GetClusterFunc()

if len(toolchainClusters) != 1 {
if !ok {
return []toolchainv1alpha1.Condition{*NewComponentErrorCondition(toolchainv1alpha1.ToolchainStatusClusterConnectionNotFoundReason, ErrMsgClusterConnectionNotFound)}
}
toolchainCluster := toolchainClusters[0]

// check conditions of cluster connection
if !cluster.IsReady(toolchainCluster.ClusterStatus) {
Expand Down
57 changes: 28 additions & 29 deletions pkg/status/toolchaincluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
t.Run("condition ready", func(t *testing.T) {
// given
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterReady(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterReady(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -51,9 +51,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
// given
msg := "the cluster connection was not found"
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterNotOk(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterNotOk(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -75,9 +75,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
t.Run("condition cluster ok but not ready", func(t *testing.T) {
// given
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterOkButNotReady(fakeToolchainClusterMsg),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterOkButNotReady(fakeToolchainClusterMsg),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -100,9 +100,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
// given
msg := "the cluster connection is not ready"
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterOkButNotReady(""),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterOkButNotReady(""),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -125,9 +125,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
// given
msg := "the cluster connection is not ready"
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterOkWithClusterOfflineCondition(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterOkWithClusterOfflineCondition(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -150,9 +150,9 @@ func TestGetToolchainClusterConditions(t *testing.T) {
// given
msg := "exceeded the maximum duration since the last probe: 13s"
readyAttrs := ToolchainClusterAttributes{
GetClustersFunc: newGetHostClusterLastProbeTimeExceeded(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
GetClusterFunc: newGetHostClusterLastProbeTimeExceeded(),
Period: 10 * time.Second,
Timeout: 3 * time.Second,
}
expected := toolchainv1alpha1.Condition{
Type: toolchainv1alpha1.ConditionReady,
Expand All @@ -173,38 +173,37 @@ func TestGetToolchainClusterConditions(t *testing.T) {
})
}

func newGetHostClusterReady() cluster.GetClustersFunc {
func newGetHostClusterReady() cluster.GetHostClusterFunc {
return NewFakeGetHostCluster(true, toolchainv1alpha1.ToolchainClusterReady, corev1.ConditionTrue, metav1.Now(), fakeToolchainClusterReason, "")
}

func newGetHostClusterNotOk() cluster.GetClustersFunc {
func newGetHostClusterNotOk() cluster.GetHostClusterFunc {
return NewFakeGetHostCluster(false, toolchainv1alpha1.ToolchainClusterReady, corev1.ConditionFalse, metav1.Now(), fakeToolchainClusterReason, fakeToolchainClusterMsg)
}

func newGetHostClusterOkButNotReady(message string) cluster.GetClustersFunc {
func newGetHostClusterOkButNotReady(message string) cluster.GetHostClusterFunc {
return NewFakeGetHostCluster(true, toolchainv1alpha1.ToolchainClusterReady, corev1.ConditionFalse, metav1.Now(), fakeToolchainClusterReason, message)
}

func newGetHostClusterOkWithClusterOfflineCondition() cluster.GetClustersFunc {
func newGetHostClusterOkWithClusterOfflineCondition() cluster.GetHostClusterFunc {
return NewFakeGetHostCluster(true, toolchainv1alpha1.ToolchainClusterOffline, corev1.ConditionFalse, metav1.Now(), fakeToolchainClusterReason, fakeToolchainClusterMsg)
}

func newGetHostClusterLastProbeTimeExceeded() cluster.GetClustersFunc {
func newGetHostClusterLastProbeTimeExceeded() cluster.GetHostClusterFunc {
tenMinsAgo := metav1.Now().Add(time.Duration(-10) * time.Minute)
return NewFakeGetHostCluster(true, toolchainv1alpha1.ToolchainClusterReady, corev1.ConditionTrue, metav1.NewTime(tenMinsAgo), fakeToolchainClusterReason, fakeToolchainClusterMsg)
}

// NewGetHostCluster returns cluster.GetHostClusterFunc function. The cluster.CachedToolchainCluster
// that is returned by the function then contains the given client and the given status and lastProbeTime.
// If ok == false, then the function returns nil for the cluster.
func NewFakeGetHostCluster(ok bool, conditionType toolchainv1alpha1.ToolchainClusterConditionType, status corev1.ConditionStatus, lastProbeTime metav1.Time, reason, message string) cluster.GetClustersFunc {
func NewFakeGetHostCluster(ok bool, conditionType toolchainv1alpha1.ToolchainClusterConditionType, status corev1.ConditionStatus, lastProbeTime metav1.Time, reason, message string) cluster.GetHostClusterFunc {
if !ok {
return func(conditions ...cluster.Condition) []*cluster.CachedToolchainCluster {
return nil
return func() (*cluster.CachedToolchainCluster, bool) {
return nil, false
}
}

return func(conditions ...cluster.Condition) []*cluster.CachedToolchainCluster {
return func() (*cluster.CachedToolchainCluster, bool) {
toolchainClusterValue := &cluster.CachedToolchainCluster{
Config: &cluster.Config{
OperatorNamespace: test.HostOperatorNs,
Expand All @@ -223,6 +222,6 @@ func NewFakeGetHostCluster(ok bool, conditionType toolchainv1alpha1.ToolchainClu
toolchainClusterValue.ClusterStatus.Conditions[0].Message = message
}

return []*cluster.CachedToolchainCluster{toolchainClusterValue}
return toolchainClusterValue, true
}
}

0 comments on commit ff9924f

Please sign in to comment.