Skip to content

Conversation

@TomatoAres
Copy link
Contributor

@TomatoAres TomatoAres commented Jul 30, 2021

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fix tke can't delete machine when delete its cluster.

  1. Modify cluster/machine needsUpdate check;
  2. Move delete cluster/machine to provider;
  3. Fix some log error;
  4. Fix patch machine finalizer context。

Which issue(s) this PR fixes:

Fixes #1351

@leoryu leoryu changed the title fix(platform):delete machine failed(#1351) fix(platform): delete machine failed Jul 30, 2021
@TomatoAres TomatoAres changed the title fix(platform): delete machine failed fix(platform): delete machine failed & move machine operation to provider Jul 30, 2021
@TomatoAres
Copy link
Contributor Author

We decide to move DeleteMachine operation from cluster controller to cluster provider.

After modified codes by design, we meet a new question:

  • Delete machine callback has deleteNode function, but some code block this operation.

After I delete this judgment, we will meet a new error:

machine:*.platform.tkestack.io "*" is forbidden: User "admin" cannot listMachines resource "machine:*" in API group "platform.tkestack.io" at the cluster scope: check cluster: cls-plmp6nm6 err: clusters.platform.tkestack.io "cls-plmp6nm6" not found

errorlog

So we need some discussion next weekdays.

@leoryu @huxiaoliang

@TomatoAres TomatoAres force-pushed the fix-1351 branch 2 times, most recently from b0d7c0d to 66ec962 Compare August 3, 2021 09:43
@pavlelee
Copy link
Collaborator

pavlelee commented Aug 3, 2021

LGTM

Copy link
Collaborator

@pavlelee pavlelee left a comment

Choose a reason for hiding this comment

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

LGTM

log.FromContext(ctx).Infof("delete machine %s failed.", machine.Name)
return err
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if machine not found, should return nil to break

if !errors.IsNotFound(err) {
return err
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

if node not found, should return nil to break

@leoryu leoryu merged commit 99117f4 into tkestack:master Aug 4, 2021
@TomatoAres TomatoAres deleted the fix-1351 branch August 4, 2021 02:34
leoryu added a commit that referenced this pull request Aug 5, 2021
leoryu added a commit that referenced this pull request Aug 5, 2021
@TomatoAres TomatoAres restored the fix-1351 branch August 5, 2021 03:54
leoryu added a commit that referenced this pull request Aug 13, 2021
leoryu added a commit that referenced this pull request Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to completely delete machine when delete cluster

3 participants