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.

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

Fix intermittent app-mobility status error #504

Merged
merged 9 commits into from
Feb 28, 2024
Merged

Fix intermittent app-mobility status error #504

merged 9 commits into from
Feb 28, 2024

Conversation

jooseppi-luna
Copy link
Contributor

@jooseppi-luna jooseppi-luna commented Feb 28, 2024 •
edited
Loading

Description

Work by @JacobGros . Basically, we fixed two minor bugs with csm status field:

  1. When we detected a failed status for a csm object, we weren't necessarily doing anything about it. Now, we requeue the reconciler if we detect a failed state.
  2. When we discover a new Succeeded state through SyncCSM, we were updating our local instance object but not writing that update anywhere. Now, we are writing the update back to the cluster by calling UpdateStatus in the HandleSuccess function.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1171

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have maintained backward compatibility

How Has This Been Tested?

Installed AM 200 times back-to-back and confirmed state was always succeeded.

@jooseppi-luna jooseppi-luna changed the base branch from main to release-v1.4.3 February 28, 2024 21:43
@jooseppi-luna jooseppi-luna merged commit 430bd76 into release-v1.4.3 Feb 28, 2024
jooseppi-luna added a commit that referenced this pull request Mar 2, 2024
@jooseppi-luna @JacobGros
Fix intermittent app-mobility status error (#504)
dba0139 
* fix standalone AM status

* add debug logs

* Revert "add debug logs"

This reverts commit 0ecb8ba.

* add debug logs back

This reverts commit e7eaa71.

* try requeue when csm.state is failed

* fix panic error

* move sync error def

* add UpdateStatus call to reconcile

* remove extra debug logs

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>
jooseppi-luna added a commit that referenced this pull request Mar 5, 2024
@jooseppi-luna @JacobGros @panigs7
Cherry-pick v1.4.3 changes into operator main branch (#515)
0067f14 
* Fix intermittent app-mobility status error (#504)

* fix standalone AM status

* add debug logs

* Revert "add debug logs"

This reverts commit 0ecb8ba.

* add debug logs back

This reverts commit e7eaa71.

* try requeue when csm.state is failed

* fix panic error

* move sync error def

* add UpdateStatus call to reconcile

* remove extra debug logs

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>

* Remove "minio" as the region for the Velero resources (#505)

* add custom region support

* add file for unit test

* remove minio hardcode from vsl

* fix unit tests for 1.9.3

* clean up import

* Add AM 1.0.2 support (#509)

* add AM v1.0.2

* update version for testing

* Update version-values.yaml (#511)

* Final manifest update (#514)

* final manifest update

* indentation fix

* remove unneeded certificate files

* fix golangci-lint issues

* update version for unit tests

* Update version-values.yaml

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>
Co-authored-by: panigs7 <92028646+panigs7@users.noreply.github.com>
ChristianAtDell added a commit that referenced this pull request Oct 15, 2024
@ChristianAtDell @JacobGros @panigs7
Cherry-pick v1.4.3 changes into operator main branch (#515)
2b69965 
* Fix intermittent app-mobility status error (#504)

* fix standalone AM status

* add debug logs

* Revert "add debug logs"

This reverts commit 0ecb8ba.

* add debug logs back

This reverts commit e7eaa71.

* try requeue when csm.state is failed

* fix panic error

* move sync error def

* add UpdateStatus call to reconcile

* remove extra debug logs

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>

* Remove "minio" as the region for the Velero resources (#505)

* add custom region support

* add file for unit test

* remove minio hardcode from vsl

* fix unit tests for 1.9.3

* clean up import

* Add AM 1.0.2 support (#509)

* add AM v1.0.2

* update version for testing

* Update version-values.yaml (#511)

* Final manifest update (#514)

* final manifest update

* indentation fix

* remove unneeded certificate files

* fix golangci-lint issues

* update version for unit tests

* Update version-values.yaml

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>
Co-authored-by: panigs7 <92028646+panigs7@users.noreply.github.com>
@mjsdell mjsdell deleted the fix-am-status branch October 17, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JacobGros JacobGros JacobGros approved these changes

@bharathsreekanth bharathsreekanth bharathsreekanth approved these changes

@abhi16394 abhi16394 Awaiting requested review from abhi16394

@mbasha-dell mbasha-dell Awaiting requested review from mbasha-dell

@alikdell alikdell Awaiting requested review from alikdell

@chimanjain chimanjain Awaiting requested review from chimanjain

@coulof coulof Awaiting requested review from coulof

@Deepak-Ghivari Deepak-Ghivari Awaiting requested review from Deepak-Ghivari

@gallacher gallacher Awaiting requested review from gallacher

@HarishH-DELL HarishH-DELL Awaiting requested review from HarishH-DELL

@karthikk92 karthikk92 Awaiting requested review from karthikk92

@kumarkgosa kumarkgosa Awaiting requested review from kumarkgosa

@bandak2 bandak2 Awaiting requested review from bandak2

@mjsdell mjsdell Awaiting requested review from mjsdell

@nitesh3108 nitesh3108 Awaiting requested review from nitesh3108

@Prabhu-Dell Prabhu-Dell Awaiting requested review from Prabhu-Dell

@rajendraindukuri rajendraindukuri Awaiting requested review from rajendraindukuri

@rajkumar-palani rajkumar-palani Awaiting requested review from rajkumar-palani

@shefali-malhotra shefali-malhotra Awaiting requested review from shefali-malhotra

@panigs7 panigs7 Awaiting requested review from panigs7

@tdawe tdawe Awaiting requested review from tdawe

@shaynafinocchiaro shaynafinocchiaro Awaiting requested review from shaynafinocchiaro

@atye atye Awaiting requested review from atye

@sharmilarama sharmilarama Awaiting requested review from sharmilarama

@shanmydell shanmydell Awaiting requested review from shanmydell

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jooseppi-luna @JacobGros @bharathsreekanth