Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor aws-network-topology-operator to put finalizer on the AWSCluster instead of Cluster #1827

Closed
1 task done
Tracked by #2737
alex-dabija opened this issue Jan 4, 2023 · 3 comments
Closed
1 task done
Tracked by #2737
Assignees
Labels
area/kaas Mission: Cloud Native Platform - Self-driving Kubernetes as a Service goal/capa-internal-ga kind/task provider/cluster-api-aws Cluster API based running on AWS team/phoenix Team Phoenix topic/capi

Comments

@alex-dabija
Copy link

alex-dabija commented Jan 4, 2023

Task

Refactor aws-network-topology-operator to put finalizer on the AWSCluster instead of Cluster.

Towards epic.

Background

Currently, the aws-network-topology-operator adds a finalizer on the Cluster resources. This leads to a race condition because in certain situations the AWSCluster resource could be deleted before the operator is ready to remove the finalizer from the Cluster resource and it still expects the AWSCluster resource to be around.

This probably doesn't happen that often because in order for the AWSCluster resource to be deleted the transit gateway attachment needs to be removed.

Update 2023-08: When implementing this, ensure #2714 (comment) is mentioned as use case, and gets fixed.

Tasks

Preview Give feedback
@alex-dabija alex-dabija changed the title refactor aws-network-topology-operator to put finalizer on the AWSCluster instead of Cluster Refactor aws-network-topology-operator to put finalizer on the AWSCluster instead of Cluster Jan 5, 2023
@alex-dabija alex-dabija added area/kaas Mission: Cloud Native Platform - Self-driving Kubernetes as a Service team/hydra topic/capi provider/cluster-api-aws Cluster API based running on AWS kind/task labels Jan 5, 2023
@bdehri bdehri self-assigned this Jan 16, 2023
@bdehri
Copy link

bdehri commented Jan 16, 2023

It looks like finalizer is put on both Cluster and AWSCluster. So, what we want to do is put the finalizer only on AWSCluster ?

func (g *Cluster) AddFinalizer(ctx context.Context, capiCluster *capi.Cluster, finalizer string) error {
  originalCluster := capiCluster.DeepCopy()
  controllerutil.AddFinalizer(capiCluster, finalizer)
  if err := g.Client.Patch(ctx, capiCluster, client.MergeFrom(originalCluster)); err != nil {
    return err
  }
  capaCluster, err := g.GetAWSCluster(ctx, types.NamespacedName{Name: capiCluster.Name, Namespace: capiCluster.Namespace})
  if err != nil {
    return err
  }
  originalCAPACluster := capaCluster.DeepCopy()
  controllerutil.AddFinalizer(capaCluster, finalizer)
  return g.Client.Patch(ctx, capaCluster, client.MergeFrom(originalCAPACluster))
}

@bdehri bdehri removed their assignment Jan 17, 2023
@alex-dabija alex-dabija self-assigned this Jan 25, 2023
@alex-dabija alex-dabija added team/phoenix Team Phoenix and removed team/hydra labels Jun 29, 2023
@fiunchinho
Copy link
Member

I think the controller should only reconcile AWSClusters and put the finalizer only to AWSClusters.

@mnitchev
Copy link
Member

mnitchev commented Jan 9, 2024

@mnitchev mnitchev closed this as completed Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kaas Mission: Cloud Native Platform - Self-driving Kubernetes as a Service goal/capa-internal-ga kind/task provider/cluster-api-aws Cluster API based running on AWS team/phoenix Team Phoenix topic/capi
Projects
None yet
Development

No branches or pull requests

5 participants