Skip to content

Conversation

@dveeden
Copy link
Contributor

@dveeden dveeden commented Oct 30, 2025

What problem does this PR solve?

Follow-up for #2565

This reduces a lot of complexity and code that isn't used or tested often.

What is changed and how it works?

Remove more Ansible import related things.

  • This is keeping the Imported field, but sets the key to -.

Questions
Should we check if the imported field is set in the config and throw an error if that's the case?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release notes:

Clusters imported from an ansible installation are not supported anymore

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 30, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xhebox for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot requested a review from kaaaaaaang October 30, 2025 08:41
@ti-chi-bot ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 30, 2025
@dveeden
Copy link
Contributor Author

dveeden commented Oct 30, 2025

/cc @xhebox

@ti-chi-bot ti-chi-bot bot requested a review from xhebox October 30, 2025 08:43
@AstroProfundis
Copy link
Contributor

TiDB-Ansible used a different dir scheme on nodes, and when importing to TiUP Cluster it's not capable to change that, so there are many compatibility wraps.

For those old legacy clusters, if these wrapping codes got removed, the cluster might be unable to start after reloading config with new tiup-cluster, (or maybe better, they fail specification validation so nothing actually change on their node, no data loss risk)

I don't know if there are still any of those old clusters left but it's better to confirm. AFAIK some on-premise deployments were of this kind, I don't know if they are still in use.

@dveeden
Copy link
Contributor Author

dveeden commented Nov 3, 2025

@AstroProfundis thanks for the info. This is why I checked "Breaking backward compatibility" and set the release note to mention this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants