Skip to content

Fix: update status when deleting and wait for modifying volumes #6164

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 25, 2025
Merged

Fix: update status when deleting and wait for modifying volumes #6164

merged 8 commits into from
Apr 25, 2025

Conversation

fgksgf
Copy link
Member

@fgksgf fgksgf commented Apr 23, 2025 •
edited
Loading

What problem does this PR solve?

  • When group/instance is deleting, should update the observed generation in the status
  • When wait for modifying volumes, also should update the observed generation in the status. Currently, it will fail the task and skip all next tasks:
| PVC           | Fail       | failed to sync pvcs: %!w(*fmt.wrapError=&{failed to modify volume's        |
|               |            | attributes tidb10172225880522700834/data-db-tikv-yu5hra: wait for volume   |
|               |            | tidb10172225880522700834/data-db-tikv-yu5hra modification completed        |
|               |            | 0xc001965180})                                                             |

What is changed and how does it work?

Code changes

  • Has Go code change
  • Has CI related scripts change

Tests

  • Unit test
  • E2E test
  • Manual test
  • No code

Side effects

  • Breaking backward compatibility
  • Other side effects:

Related changes

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

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.

@ti-chi-bot ti-chi-bot bot requested a review from howardlau1999 April 23, 2025 03:31
@github-actions github-actions bot added the v2 for operator v2 label Apr 23, 2025
@github-actions GitHub Actions
Copy link

/run-pull-e2e-kind-v2

@ti-chi-bot ti-chi-bot bot added the size/XXL label Apr 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 45.45455% with 78 lines in your changes missing coverage. Please review.

Project coverage is 51.82%. Comparing base (655bb03) to head (42ca87a).

Additional details and impacted files 
@@              Coverage Diff               @@
##           feature/v2    #6164      +/-   ##
==============================================
- Coverage       52.08%   51.82%   -0.27%     
==============================================
  Files             232      232              
  Lines           16355    16399      +44     
==============================================
- Hits             8518     8498      -20     
- Misses           7837     7901      +64
Flag Coverage Δ
unittest 51.82% <45.45%> (-0.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fgksgf fgksgf force-pushed the huaxi/update-status branch from a3f8dd7 to 19a3bf7 Compare April 23, 2025 04:25
@github-actions GitHub Actions
Copy link

/run-pull-e2e-kind-v2

liubog2008
liubog2008 reviewed Apr 23, 2025
View reviewed changes
pkg/apiutil/core/v1alpha1/group.go Outdated
@@ -21,6 +21,7 @@ import (
"github.com/pingcap/tidb-operator/pkg/client"
"github.com/pingcap/tidb-operator/pkg/runtime"
"github.com/pingcap/tidb-operator/pkg/runtime/scope"
"github.com/pingcap/tidb-operator/pkg/utils"
Copy link
Member

Choose a reason for hiding this comment

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

Do not add function to utils directly. Use utils/xxx instead.

liubog2008
liubog2008 reviewed Apr 23, 2025
View reviewed changes
pkg/controllers/ticdc/builder.go
@@ -37,6 +37,7 @@ func (r *Reconciler) NewRunner(state *tasks.ReconcileContext, reporter task.Task

task.IfBreak(common.CondInstanceIsDeleting(state),
tasks.TaskFinalizerDel(state, r.Client),
common.TaskStatusPersister[scope.TiCDC](state, r.Client),
Copy link
Member

Choose a reason for hiding this comment

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

no need to change any status?

@github-actions GitHub Actions
Copy link

/run-pull-e2e-kind-v2

liubog2008
liubog2008 reviewed Apr 25, 2025
View reviewed changes
api/core/v1alpha1/common_types.go
@@ -301,6 +301,15 @@ type CommonStatus struct {
CollisionCount *int32 `json:"collisionCount,omitempty"`
}

// StoreStatus defines the common status fields for all stores.
type StoreStatus struct {
Copy link
Member

Choose a reason for hiding this comment

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

It seems not used?

@github-actions GitHub Actions
Copy link

/run-pull-e2e-kind-v2

@liubog2008
Copy link
Member

/lgtm

@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Apr 25, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liubog2008

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

The pull request process is described 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
Copy link
Contributor

ti-chi-bot bot commented Apr 25, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-04-25 08:35:37.72609081 +0000 UTC m=+603881.537881192: ☑️ agreed by liubog2008.

@ti-chi-bot ti-chi-bot bot merged commit b618aba into pingcap:feature/v2 Apr 25, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liubog2008 liubog2008 liubog2008 left review comments

@howardlau1999 howardlau1999 Awaiting requested review from howardlau1999

Assignees
No one assigned
Labels
approved lgtm size/XXL v2 for operator v2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fgksgf @codecov-commenter @liubog2008