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

Update volumesource to be json inline #577

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

dprince
Copy link
Contributor

@dprince dprince commented Nov 1, 2024

No description provided.

@dprince
Copy link
Contributor Author

dprince commented Nov 1, 2024

testing this fix here: openstack-k8s-operators/openstack-operator#1158

@dprince dprince force-pushed the volume_inline_fix branch 2 times, most recently from 0d9a5e1 to 8d52793 Compare November 1, 2024 18:44
// VolumeSource defines the source of a volume to be mounted
VolumeSource VolumeSource `json:"volumeSource"`
VolumeSource VolumeSource `json:",inline" protobuf:"bytes,2,opt,name=volumeSource"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still need a small change of L103. As per my tests (in manila-operator) it should be:

VolumeSource `json:",inline" protobuf:"bytes,2,opt,name=volumeSource"``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!. Pushed a fix.

Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 71a0e9d into openstack-k8s-operators:main Nov 4, 2024
2 checks passed
@fmount
Copy link
Contributor

fmount commented Nov 4, 2024

Testing was still in progress but early results in manila-operator show that this fix works as expected (LGTM as well)

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