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

Bug 2089797: Cache S3 errors from UploadPV (and other MDR fixes) #44

Merged

Conversation

ShyamsundarR
Copy link

Backporting final fix for Bug 2089797 additionally following fixes for MetroDR are also included:

  • Move fencing CRD dependency to the appropriate repository
  • Add ManagedClusterView to ensure fencing operation is complete before failing over
  • Volsync to check volume attachements for PVC usage before final sync

kseegerrh and others added 19 commits June 21, 2022 09:33
Signed-off-by: Karolin Seeger <kseeger@redhat.com>
(cherry picked from commit bb4b993)
only when all DRPCs are deleted

Signed-off-by: Jolly Mishra <jmishra@redhat.com>
(cherry picked from commit ee61738)
Signed-off-by: Tesshu Flower <tflower@redhat.com>
(cherry picked from commit f33fbf6)
Signed-off-by: hatfieldbrian <bhatfiel@redhat.com>
(cherry picked from commit feff50f)
	add to ObjectStorer interface:
		- UploadObject()
		- DownloadObject()
	remove from ObjectStorer interface:
		- UploadPV()
		- DownloadPVs()
	remove from s3ObjectStore interface:
		- UploadTypedObject()
		- VerifyPVUpload()
		- DownloadTypedObjects()
	remove:
		- ObjectStorer.GetName()
	modify fakeObjectStorer implementation:
		- store objects in a map
		- maintain one map per profile
		- DeleteObjects() requires specified prefix match

Signed-off-by: hatfieldbrian <bhatfiel@redhat.com>
(cherry picked from commit 0043954)
Initially drclusterUpdater was mainly for updating the
DRCluster status. However, since the structure is used
for other functionalities such as fencing/unfencing,
rename the structure into an instance.

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
(cherry picked from commit 0f84a7f)
* Initialize the status only when status cannot be found
  in the fetched resource from reconciler. This will avoid
  some reconciles happening just because of status update.

* Update status only when current status is different from
  the previously saved status.

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
(cherry picked from commit ae7cb92)
Signed-off-by: Raghavendra M <raghavendra@redhat.com>
(cherry picked from commit 5800160)
* Update the required version of k8s.io/client-go package
  to 0.23.6 to maintain compatibility with csiaddons

* Bump up the required go version from 1.16 to 1.17

Co-authored-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra M <raghavendra@redhat.com>
(cherry picked from commit d953bff)
For VRG, when the reconcile begins, status conditions
are not initialized if the length of the conditions is
greater than 0. This check can sometimes miss the case
where one of the conditions is not there in the status,
and the check assumes everything is correct.

This commit changes it to check explicitly for 4 conditions
for not initializing the conditions. Otherwise, it initializes
a condition only if that particular condition type is not
found in the status.

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
Currently the way kubebuilder is defined in DRCluster types,
it does not consider ManuallyUnfenced option. It makes it
difficult to consume the option while creating/updating the
DRCluster resource.

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
Add tests for failover/relocation in syncDR for
manual fencing and unfencing

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
Signed-off-by: hatfieldbrian <bhatfiel@redhat.com>
Signed-off-by: hatfieldbrian <bhatfiel@redhat.com>
- During relocate, before running a final sync - keep the check to
  ensure no pod is mounting the PVC - if no pod is, then do an
  additional check of checking for volumeattachments to make sure
  the underlying PV has been unmounted from nodes (and therefore I/O
  writes complete).  If a CSI driver does not support volumeattachments
  then this check will not find any, and will assume the PVC is
  unmounted.

Signed-off-by: Tesshu Flower <tflower@redhat.com>
DRCluster creates the NetworkFence resource in the managed cluster
to fence/unfence a cluster. While some of the information included
in the NetworkFence resource at the time of create/update is known
from the Spec of the DRCluster resource (like fence state, CIDRs),
there are other storage specific information that NetworkFence need
to know such as storage driver, storage parameters, secrets to be
able to communicate with the storage layer. That information is not
part of DRCluster spec. Hence, for now, DRCluster gets that data
from its annotations. In future it should be able to get that data
from its status.

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
Initial cache of errors built in commit e7b7b9d did
not account for errors faced during uploads. Emperically
it is seen that connection errors are generated during
the upload phase than the intial session creation phase.

This commit adds updating the cache with S3 errors during
the upload as well.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
* Currently MCVGetter interface can only get the MCV resource
  for VolumeReplicationGroup and Namespace resources. Added the
  functionality to get NetworkFence resource

* Moved the MCVGetter code to a separate util file called mcv_util.go

* Corresponding changes in DRPC to consume the MCVGetter from util

* Consumption of MCV in DRCluster for NetworkFence resource

Signed-off-by: Raghavendra M <raghavendra@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Jun 21, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ShyamsundarR

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

@openshift-ci
Copy link

openshift-ci bot commented Jun 21, 2022

@ShyamsundarR: This pull request references Bugzilla bug 2089797, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (ODF 4.11.0) matches configured target release for branch (ODF 4.11.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @keesturam

In response to this:

Bug 2089797: Cache S3 errors from UploadPV (and other MDR fixes)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented Jun 21, 2022

@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam.

Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@ShyamsundarR: This pull request references Bugzilla bug 2089797, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (ODF 4.11.0) matches configured target release for branch (ODF 4.11.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @keesturam

In response to this:

Bug 2089797: Cache S3 errors from UploadPV (and other MDR fixes)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ShyamsundarR ShyamsundarR merged commit 74b9644 into red-hat-storage:release-4.11 Jun 21, 2022
@ShyamsundarR ShyamsundarR deleted the ds-backport-411-cfreeze branch June 21, 2022 15:08
@openshift-ci
Copy link

openshift-ci bot commented Jun 21, 2022

@ShyamsundarR: All pull requests linked via external trackers have merged:

Bugzilla bug 2089797 has been moved to the MODIFIED state.

In response to this:

Bug 2089797: Cache S3 errors from UploadPV (and other MDR fixes)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants