Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Conversation

keithpine
Copy link
Contributor

@keithpine keithpine commented Jul 14, 2022

Description

Cleanup Data chart values, in the same vein as previous changes.

  • Rename object from cortxdata to data
  • Use camel case for variable names
  • Use new image style
  • Etc.

This has significant changes in regards to the storage set definitions. The storage sets are now defined as a top level list, in the exact same format as solution.yaml except for:

  • Key names use camel case
  • The nodes key is removed since it is not being used. Instead, the data.replicaCount defines the number of nodes

The logic to construct the CVG groups and determine the number of StatefulSets has been changed to use Helm (Sprig's really) chunk function, which groups all the CVG objects into the storage set's containerGroupSize value. This grouping means less manual calculation of counts, and no need for tracking index values.

There is still only support for a single storage set, the template errors out if there's more than one.

The block device PV access modes have been changed to ReadWriteOnce by default. ReadWriteMany isn't necessary.

Port definitions have been removed from the data headless Service. It was not worth the effort to properly handle the dynamic nature of the CVGs, and none of the other headless services specify anything. The port definitions are merely documentation not functional. We can decide to add them back later if necessary.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds new functionality)
  • Breaking change (bug fix or new feature that breaks existing functionality)
  • Third-party dependency update
  • Documentation additions or improvements
  • Code quality improvements to existing code or test additions/updates

Applicable issues

  • This change fixes an issue: CORTX-32157

How was this tested?

Deployment on a 2-node cluster with two CVGs, and using both group sizes of 1 and 2, with S3/IO. Ran status script.

I capture the Cluster state using kubectl get deployments,statefulsets,services,cm -o yaml, for both group size cases, and for before and after this change. Everything was identical in the "after" case except for the expected:

  • Removal of ports from headless server
  • Change of accessMode for block devices to ReadWriteOnce
  • Unique metadata

All volumes, PVCs, Pods, etc. were identical.

Additional information

Checklist

  • The change is tested and works locally.
  • New or changed settings in the solution YAML are documented clearly in the README.md file.
  • All commits are signed off and are in agreement with the CORTX Community DCO and CLA policy.

If this change requires newer CORTX or third party image versions:

  • The image fields in solution.example.yaml have been updated to use the required versions.
  • The appVersion field of the Helm chart has been updated to use the new CORTX version.

If this change addresses a CORTX Jira issue:

  • The title of the PR starts with the issue ID (e.g. CORTX-XXXXX:)

View rendered charts/cortx/README.md

@cla-bot cla-bot bot added the cla-signed label Jul 14, 2022
@keithpine keithpine marked this pull request as ready for review July 14, 2022 01:42
@keithpine keithpine requested a review from a team as a code owner July 14, 2022 01:42
Copy link
Contributor

@walterlopatka walterlopatka left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@osowski osowski left a comment

Choose a reason for hiding this comment

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

lgtm

@osowski osowski merged commit adc624a into Seagate:integration Jul 14, 2022
@keithpine keithpine deleted the CORTX-32157_data-refactor branch July 19, 2022 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants