Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cmd/gke-gcloud-auth-plugin/cred_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ var (
"extra_args": "%s"
}
`
invalidCacheFile = "invalid_cache_file"
fakeCurrentContext = "gke_user-gke-dev_us-east1-b_cluster-1"
cachedAccessToken = "ya29.cached_token"

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/nodeipam/ipam/cloud_cidr_allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,11 @@ func (ca *cloudCIDRAllocator) AllocateOrOccupyCIDR(node *v1.Node) error {
}

func (ca *cloudCIDRAllocator) runWorker(ctx context.Context) {
for ca.processNextItem(ctx) {
for ca.processNextItem() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to make the parameter unamed in processNextItem but keep the context.Context parameter. It is best practice to propagate the Context and we should keep this in case there is a future change that will use it.

}
}

func (ca *cloudCIDRAllocator) processNextItem(ctx context.Context) bool {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not remove this, it can also be used for context logging

func (ca *cloudCIDRAllocator) processNextItem() bool {
key, quit := ca.queue.Get()
if quit {
return false
Expand Down
28 changes: 3 additions & 25 deletions pkg/controller/service/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func (c *Controller) processServiceCreateOrUpdate(ctx context.Context, service *
// This happens only when a service is deleted and re-created
// in a short period, which is only possible when it doesn't
// contain finalizer.
if err := c.processLoadBalancerDelete(ctx, cachedService.state, key); err != nil {
if err := c.processLoadBalancerDelete(ctx, cachedService.state); err != nil {
return err
}
}
Expand Down Expand Up @@ -524,12 +524,6 @@ func (s *serviceCache) getOrCreate(serviceName string) *cachedService {
return service
}

func (s *serviceCache) set(serviceName string, service *cachedService) {
s.mu.Lock()
defer s.mu.Unlock()
s.serviceMap[serviceName] = service
}

func (s *serviceCache) delete(serviceName string) {
s.mu.Lock()
defer s.mu.Unlock()
Expand Down Expand Up @@ -921,14 +915,14 @@ func (c *Controller) processServiceDeletion(ctx context.Context, key string) err
return nil
}
klog.V(2).Infof("Service %v has been deleted. Attempting to cleanup load balancer resources", key)
if err := c.processLoadBalancerDelete(ctx, cachedService.state, key); err != nil {
if err := c.processLoadBalancerDelete(ctx, cachedService.state); err != nil {
return err
}
c.cache.delete(key)
return nil
}

func (c *Controller) processLoadBalancerDelete(ctx context.Context, service *v1.Service, key string) error {
func (c *Controller) processLoadBalancerDelete(ctx context.Context, service *v1.Service) error {
// delete load balancer info only if the service type is LoadBalancer
if !WantsLoadBalancer(service) {
return nil
Expand Down Expand Up @@ -1003,12 +997,6 @@ func (c *Controller) patchStatus(service *v1.Service, previousStatus, newStatus
type NodeConditionPredicate func(node *v1.Node) bool

var (
allNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{
nodeIncludedPredicate,
nodeUnTaintedPredicate,
nodeReadyPredicate,
}

stableNodeSetPredicates []NodeConditionPredicate = []NodeConditionPredicate{
nodeNotDeletedPredicate,
nodeIncludedPredicate,
Expand Down Expand Up @@ -1044,16 +1032,6 @@ func nodeUnTaintedPredicate(node *v1.Node) bool {
return true
}

// We consider the node for load balancing only when its NodeReady condition status is ConditionTrue
func nodeReadyPredicate(node *v1.Node) bool {
for _, cond := range node.Status.Conditions {
if cond.Type == v1.NodeReady {
return cond.Status == v1.ConditionTrue
}
}
return false
}

func nodeNotDeletedPredicate(node *v1.Node) bool {
return node.DeletionTimestamp.IsZero()
}
Expand Down