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 18, 2022

Description

This change updates the solution.yaml storage set definition to support "log" device entries, which are described in the latest cluster.yaml sample file. The format of the solution file has changed, see the Breaking change section. Log devices are specified as a list of devices, the same as data devices.

For now, this device type is optional. Omitting or including the log devices are both supported. The solution validation will not flag missing values.

I have not included a "name" field with the devices, as suggested. We can add this later in the future, but there is currently no use for it, so there is nothing for the deployment script or Chart to use it for, thus it should not be a required field.

With images 859 (planned v0.9.0 version) and the latest 867, I can see that the cluster.conf correctly sets the log entries. I don't believe Motr uses it for anything though. I am unable to verify that.

Breaking change

The devices format of the storage sets has changed. Note that 0.9.0 already has a breaking change with respect to the formatting, so this can be included with that.

The metadata object used to take a singular device, now it is a list of devices. Note: motr setup will fail if more than one device is specified.

The device field of the device object is now more aptly named path.

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-33050
  • This change is related to an issue: CORTX-32810

CORTX image version requirements

There aren't any versions that support log devices yet. The current version, 859, works with or without them.

How was this tested?

I deployed in multiple ways:

  • Image 859 with no log device
  • Image 859 with a log device
  • Image 867 with no log device
  • Image 867 with a log device

In all cases, the behavior was the same: the cluster deployed successfully and scripts, S3, etc. were working. When log devices are specified, the statefulsets correctly mount all of the volume devices (meta + log + data), and the cluster.conf file is generated with an entry.

solution.yaml

      devices:
        metadata:
        - path: /dev/sdc
          size: 25Gi
        log:
        - path: /dev/sdd
          size: 25Gi
        data:
        - path: /dev/sde
          size: 25Gi

cluster.yaml

    storage:
    - name: cvg-1
      type: ios
      devices:
        metadata:
        - /dev/sdc
        log:
        - /dev/sdd
        data:
        - /dev/sde

cluster.conf

    cvg:
    - devices:
        data:
        - /dev/sde
        log:
        - /dev/sdd
        metadata:
        - /dev/sdc
        num_data: 1
        num_log: 1
        num_metadata: 1
      name: cvg-1
      type: ios

The status script passes, as it verifies the number of PV and PVCs are created according to the solution file.

Additional information

I have not added the log device entry to the README.md file or chart values.yaml file yet. It should be added when the feature is ready to be used.

Fix list formatting in cluster.yaml to match the sample and be more consistent.

Remove the common.release key from config.yaml since this matches the sample.

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:)

@cla-bot cla-bot bot added the cla-signed label Jul 18, 2022
@keithpine keithpine marked this pull request as ready for review July 18, 2022 22:37
@keithpine keithpine requested a review from a team as a code owner July 18, 2022 22:37
@keithpine keithpine requested review from tanujashinde0405 and removed request for a team July 18, 2022 22:37
@keithpine
Copy link
Contributor Author

The metadata object used to take a singular device, now it is a list of devices. Note: motr setup will fail if more than one device is specified.

Not sure if we should do anything about this? The list format was suggested in the Jira ticket, and it also makes the handling of the devices more consistent. Should I remove more than the 1st from the deployment script? Ignore all but the first in the Chart? Just document it in the README?

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

@keithpine keithpine merged commit 480d986 into Seagate:integration Jul 19, 2022
@keithpine keithpine deleted the CORTX-33050_log-devices branch July 19, 2022 14:18
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