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

NFS storage concept #842

Merged
merged 6 commits into from
Oct 17, 2023
Merged

NFS storage concept #842

merged 6 commits into from
Oct 17, 2023

Conversation

pbochynski
Copy link
Contributor

@pbochynski pbochynski commented Sep 25, 2023

Description

Changes proposed in this pull request:

  • concept for NFS storage in Kyma runtime

@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2023
@kyma-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kyma-bot kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 25, 2023
@CLAassistant
Copy link

CLAassistant commented Oct 12, 2023

CLA assistant check
All committers have signed the CLA.

@pbochynski pbochynski marked this pull request as ready for review October 12, 2023 14:22
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 12, 2023
@pbochynski pbochynski changed the title NFS storage concept draft NFS storage concept Oct 17, 2023
@anishj0shi
Copy link
Member

anishj0shi commented Oct 17, 2023

i quickly tested gardener's reconciliation behavior by scheduling an azure storage account in a VNet. and the standard reconciliation of clusters works without any errors.
But deletion of cluster fails due to private endpoint created in the shoot subnet.

deleting Subnet: (Name "shoot--ccloud-poc--ncm-poc-nodes" / Virtual Network Name "shoot--ccloud-poc--ncm-poc" / Resource Group "shoot--ccloud-poc--ncm-poc"):
 network.SubnetsClient#Delete: Failure sending request: StatusCode=400 -- Original Error: Code="InUseSubnetCannotBeDeleted" 
Message="Subnet shoot--ccloud-poc--ncm-poc-nodes is in use by /subscris/ENDPOINT-STORAGE.NIC.<omitted>/ipConfigurations/PRIVATEENDPOINTIPCONFIG.<omitted> 
and cannot be deleted. 
In order to delete the subnet, delete all the resources within the subnet. See aka.ms/deletesubnet." Details=[]]

this creates a pre-requisite condition for us that, we'd need to clean up the storage account and the private endpoint in the shoot network, before we delete the cluster. if we agree to this, then we can merge this PR.

@pbochynski
Copy link
Contributor Author

i quickly tested gardener's reconciliation behavior by scheduling an azure storage account in a VNet. and the standard reconciliation of clusters works without any errors. But deletion of cluster fails due to private endpoint created in the shoot subnet.
this creates a pre-requisite condition for us that, we'd need to clean up the storage account and the private endpoint in the shoot network, before we delete the cluster. if we agree to this, then we can merge this PR.

Yes - we can assume that our controllers will listen to the GardenerCluster resource and we will delete all the resources created by our controllers. It is mentioned already in the document:

 storage instance can block cluster deprovisioning (not a big deal as storage controller in the KCP should clean it up anyway)

Copy link
Member

@anishj0shi anishj0shi left a comment

Choose a reason for hiding this comment

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

/lgtm


**Cons:**
- if clusters are deleted and created with different shoot name we can face [issues](https://cloud.google.com/filestore/docs/create-instance-issues#system_limit_for_internal_resources_has_been_reached_error_when_creating_an_instance) with private network quota for Filestore (it is not a common usage pattern)
- storage instance can block cluster deprovisioning (not a big deal as storage controller in the KCP should clean it up anyway)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- storage instance can block cluster deprovisioning (not a big deal as storage controller in the KCP should clean it up anyway)
- storage instances and private endpoints in the shoot subnet can block cluster de-provisioning (not a big deal as storage controller in the KCP should clean them up anyway)

@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 17, 2023
@kyma-bot kyma-bot merged commit 501ed90 into kyma-project:main Oct 17, 2023
1 check passed
@anishj0shi anishj0shi mentioned this pull request Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Looks good to me! size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants