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

cleanup: Support matrix update and code cleanup #4262

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

karthik-us
Copy link
Collaborator

@karthik-us karthik-us commented Nov 16, 2023

  • Updating the support matrix to point to the suppotred ceph release.
  • Cleanup the cephfs resize code.

Fixes: #4168

@karthik-us
Copy link
Collaborator Author

@Madhu-1 @Rakshith-R PTAL.

@mergify mergify bot added the cleanup label Nov 16, 2023
@karthik-us
Copy link
Collaborator Author

Note to self: Link the upstream issue in the next update.

README.md Outdated
| | Creating and deleting snapshot | Alpha | >= v3.7.0 | >= v1.1.0 | Pacific (>=16.2.0) | >= v1.17.0 |
| | Provision volume from snapshot | Alpha | >= v3.7.0 | >= v1.1.0 | Pacific (>=16.2.0) | >= v1.17.0 |
| | Provision volume from another volume | Alpha | >= v3.7.0 | >= v1.1.0 | Pacific (>=16.2.0) | >= v1.16.0 |
| RBD | Dynamically provision, de-provision Block mode RWO volume | GA | >= v1.0.0 | >= v1.0.0 | Pacific (>=v16.0.0) | >= v1.14.0 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feature Status need to be updated to change the status to next version based on the support we have (This is for followup PR)

unsupported
)

type localClusterState struct {
// set the enum value i.e., unknown, supported,
// unsupported as per the state of the cluster.
resizeState operationState
subVolMetadataState operationState
subVolSnapshotMetadataState operationState
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove subVolSnapshotMetadataState as well?

can you also check do we have metadata support in v16.0.0 as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

metadata & snapshot metadata support are added on pacific from v16.2.11 and on quincy it is from v17.2.1. Should we keep both these checks for some more time in that case, if someone uses versions lower than that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If that is a supported version in ceph we need to keep it as well. ie (< 16.2.11) and (<17.2.1)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As per the docs 16.2.* and 17.2.* are still supported. So we need to keep these checks for some more time.
Will update support matrix as well to point to >=v16.2.0 for all the pacific versions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i believe in ResizeSubVolume we can remove the check and also remove the call for createVolume. can you please check on that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the support for it is present in all the active releases. Removed it now.

@@ -199,16 +199,13 @@ func (s *subVolumeClient) GetSubVolumeInfo(ctx context.Context) (*Subvolume, err
type operationState int64

const (
unknown operationState = iota
supported
supported operationState = iota
unsupported
)

type localClusterState struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

localClusterState might need to be removed as well as we don't require it anymore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the checks are needed, keeping it as it is for the time being.

// ResizeVolume will try to use ceph fs subvolume resize command to resize the
// subvolume. If the command is not available as a fallback it will use
// CreateVolume to resize the subvolume.
// ResizeVolume will use ceph fs subvolume resize command to resize the subvolume.
func (s *subVolumeClient) ResizeVolume(ctx context.Context, bytesQuota int64) error {
newLocalClusterState(s.clusterID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

newLocalClusterState(s.clusterID) also need to be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@karthik-us karthik-us changed the title cleanup: Support matrix update and code cleanup doc: Support matrix update Nov 17, 2023
@mergify mergify bot added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/docs Issues and PRs related to documentation labels Nov 17, 2023
@karthik-us karthik-us force-pushed the supported-version-changes branch 2 times, most recently from c106681 to 82cf777 Compare November 17, 2023 14:03
@karthik-us karthik-us changed the title doc: Support matrix update cleanup: Support matrix update and code cleanup Nov 17, 2023
@karthik-us
Copy link
Collaborator Author

/test ci/centos/mini-e2e/k8s-1.28

@karthik-us
Copy link
Collaborator Author

@Rakshith-R @nixpanic @yati1998 PTAL.

Copy link
Contributor

@iPraveenParihar iPraveenParihar left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@Rakshith-R
Copy link
Contributor

@karthik-us
Can you please update PendingRelease.md too ?

@karthik-us
Copy link
Collaborator Author

@karthik-us Can you please update PendingRelease.md too ?

@Rakshith-R just the mention of removal of resize code with the reason suffice in the pending release notes as the desscription for the PR right? under the Breaking Chnages.

@Rakshith-R
Copy link
Contributor

@karthik-us Can you please update PendingRelease.md too ?

@Rakshith-R just the mention of removal of resize code with the reason suffice in the pending release notes as the desscription for the PR right? under the Breaking Chnages.

The code change is internal only.
No, I meant regarding the support status change for ceph version.

@mergify mergify bot dismissed iPraveenParihar’s stale review November 22, 2023 11:05

Pull request has been modified.

@karthik-us
Copy link
Collaborator Author

@karthik-us Can you please update PendingRelease.md too ?

@Rakshith-R just the mention of removal of resize code with the reason suffice in the pending release notes as the desscription for the PR right? under the Breaking Chnages.

The code change is internal only. No, I meant regarding the support status change for ceph version.

Done.

@nixpanic nixpanic removed ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures labels Nov 23, 2023
@nixpanic
Copy link
Member

@Mergifyio rebase

Updating the support matrix to point to the suppotred ceph release.

Fixes: ceph#4168
Signed-off-by: karthik-us <ksubrahm@redhat.com>
The ceph fs subvolume resize support is available
in all the active ceph releases. Hence removing the
code to check the supportability of the feature.

Signed-off-by: karthik-us <ksubrahm@redhat.com>
Adding a note on supported ceph releases.

Signed-off-by: karthik-us <ksubrahm@redhat.com>
Copy link
Contributor

mergify bot commented Nov 23, 2023

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Nov 23, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b5d8bf0

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Nov 23, 2023
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Nov 23, 2023
@mergify mergify bot merged commit b5d8bf0 into ceph:devel Nov 23, 2023
34 checks passed
@karthik-us karthik-us deleted the supported-version-changes branch November 24, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup component/docs Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove support for older kubernetes/ceph releases
6 participants