Implement support for hostpath solr data volumes. #265
Conversation
|
@HoustonPutman I'm not sure what this error means, any ideas? |
That is saying that you have two extra lines in your go.mod. Try running |
|
They don't go away. Should I just remove the changes to go.mod? |
|
That's a good idea. I pulled your PR and ran |
|
Interestingly I removed both and re-ran make mod-tidy, and the |
jward-bw
force-pushed
the
implement-support-for-hostpath-solr-data-volumes
branch
from
65f10b7 to
7a9a717
Compare
|
I don't use go regularly, so I shouldn't have any weird config. I did recently update to the latest version, so I wonder if that's caused a change in behaviour for me? 🤷 |
HoustonPutman
left a comment
HoustonPutman
left a comment
There was a problem hiding this comment.
This looks very good. One small issue with an empty ephemeral storage options. Not sure if it could actually happen, depending on how go parses the yaml/json, but still good to protect against anyways.
Also would you mind adding this option to the "Data Storage" section in docs/solr-cloud/solr-cloud-crd.md?
jward-bw
force-pushed
the
implement-support-for-hostpath-solr-data-volumes
branch
from
7a9a717 to
c78b2b8
Compare
| err = testClient.Create(context.TODO(), instance) | ||
| g.Expect(err).NotTo(gomega.HaveOccurred()) | ||
| defer testClient.Delete(context.TODO(), instance) | ||
| g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCloudRequest))) |
There was a problem hiding this comment.
You might want to copy this line twice. So two or three of the same line in a row. I think the tests will be flakey otherwise. This will help the failure that you see in the github actions.
There was a problem hiding this comment.
(Also no need to force push each time, I'll squash-merge in the end.)
There was a problem hiding this comment.
Ok thanks, I've been on repos before where they like to have everything in a single commit :)
jward-bw
force-pushed
the
implement-support-for-hostpath-solr-data-volumes
branch
from
c78b2b8 to
4a50381
Compare
HoustonPutman
left a comment
HoustonPutman
left a comment
There was a problem hiding this comment.
The looks good to me!
One final request:
Would you mind adding this option to the "Data Storage" section in docs/solr-cloud/solr-cloud-crd.md?
Done! Sorry I missed that yesterday. |
HoustonPutman
left a comment
HoustonPutman
left a comment
There was a problem hiding this comment.
Looks good! Thanks for the great contribution!
2 participants
This PR adds a new option for ephemeral data storage. Instead of the emptyDir you can use a
hostPathvolume. The default behaviour for ephemeral storage is still emptyDir, but specifyingHostPathwill mean hostPath will be used instead of emptyDir.Addresses #266