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

Edge-Node-Cluster cleanup KubeVolumeInfo and KubeStorageInfo #67

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

andrewd-zededa
Copy link
Contributor

  • StorageHealthStatus should be under KubeVolumeInfo as a substatus.
  • New enum ServiceStatus more appropriately describes multi-node service health.
  • Added missing enum entries to StorageVolumeReplicaStatus

@andrewd-zededa
Copy link
Contributor Author

Moved a field from KubeStorageInfo to KubeVolumeInfo but I'm hoping it should be ok as these should not be in use yet as they were just added recently in PR #63

StorageHealthStatus should be under KubeVolumeInfo as a substatus.
New enum ServiceStatus more appropriately describes
multi-node service health.
Added missing enum entries to StorageVolumeReplicaStatus

Signed-off-by: Andrew Durbin <[email protected]>
Signed-off-by: Andrew Durbin <[email protected]>
@andrewd-zededa
Copy link
Contributor Author

latest push should resolve yetus enum prefix issue

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM.
As you say, if there is not yet any use of KubeStorageInfo.health neither in EVE nor in any controller, then there isn't a problem doing this protobuf type change in. Did you check all of those?

@andrewd-zededa
Copy link
Contributor Author

@eriknordmark I believe that eve-api version has been pulled into a dev branch of a controller but I don't think it has been pushed to a release of a production facing controller yet. I think @xyuria-zededa can help confirm.

@xyuria-zededa
Copy link

@eriknordmark I believe that eve-api version has been pulled into a dev branch of a controller but I don't think it has been pushed to a release of a production facing controller yet. I think @xyuria-zededa can help confirm.

I haven't used it yet, so you can feel free to change anything there.

@eriknordmark eriknordmark merged commit 554b224 into lf-edge:main Sep 25, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants