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

Conversation

keithpine
Copy link
Contributor

Description

For Control and HA Pods, replace the random UUID machine-id/cluster node IDs, and use the Pod hostname instead, the same way as the Server, Data and Client Pods are done. This simplifies deployment by removing some required values, and makes a helm upgrade based Chart upgrade safer since it removes the value randomness.

All environment variables and volume mounts are removed. Provisioner uses the Pod hostname instead.

Note that the cortx.io/machine-id Pod labels have been preserved. The HA k8s monitor is using this label (although it appears only to consider the Control Pod and not its own Pod).

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 and other scripts
  • S3 I/O
  • CSM API requests

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 7, 2022
@keithpine keithpine marked this pull request as ready for review July 7, 2022 01:00
@keithpine keithpine requested a review from a team as a code owner July 7, 2022 01:00
@keithpine
Copy link
Contributor Author

@tanujashinde0405 Do you know if my description of how this works is accurate? Any problems taking this approach for Control and HA?

@tanujashinde0405
Copy link

@tanujashinde0405 Do you know if my description of how this works is accurate? Any problems taking this approach for Control and HA?

Yes this looks good to me, We already have support for converting hostnames into UUID in provisioner, So removing machine id's from here should work. Have you tried deployment with this changes, Let me know if any issues. Thanks.

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.

From everything I can tell, this should work as expected. I ran a couple deployments to validate my questions and everything came up online/green/okay. I'll defer on some of the inner workings to @tanujashinde0405 or someone from the HA/Hare team, but from my perspective at the moment, I am good.

lgtm

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 2d37657 into Seagate:integration Jul 7, 2022
@keithpine keithpine deleted the CORTX-32157_no-machine-id branch July 7, 2022 19:07
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.

4 participants