Adding vfs_fileid changes to the samba-operator #129
Conversation
We shouldn't have any scenario where we could enable printing on the operator. Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
internal/smbcc/container_config.go
Outdated
| No = "no" | ||
|
|
||
| // AddVFSFileid is used to chec if we add vfs_fileid to the config | ||
| AddVFSFileid = true |
There was a problem hiding this comment.
This doesn't seem like the right approach to me, why is this a global rather than say, an argument to NewGlobals?
There was a problem hiding this comment.
I've added a new commit which changes the way we pass the configuration options. Let me know if this is what you had in mind. Not squashing patches until they are acceptable.
|
Switched to Draft PR. |
internal/smbcc/container_config.go
Outdated
| ) | ||
|
|
||
| // Constructor for the OperatorOptions struct | ||
| func NewOperatorOptions() *OperatorOptions { |
There was a problem hiding this comment.
This is more of a taste thing I think but I'd drop this function and simply not provide a "constructor". I'd also make it a pass-by-value (not a pointer). If you want me to elaborate on why please ask, but I'm lazy (and still on vacation ;-)) so I'm not going to elaborate yet. However, it's minor and I could easily live with it as-is.
There was a problem hiding this comment.
Hah. Sorry, I didn't realise that you would be looking at patches today. I have just posted another one.
Please excuse my changes as I navigate go lang. Let's schedule a 1-1 tomorrow to go through the code.
spuiuk
force-pushed
the
fileid
branch
3 times, most recently
from
39b3e4d to
3d5db38
Compare
Provide an option to add vfs_fileid to the smb.conf Globals section. Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
|
John, |
phlogistonjohn
left a comment
phlogistonjohn
left a comment
There was a problem hiding this comment.
On first glance it looks OK, but I'll give it a more thorough reading during the work week.
Also @raghavendra-talur PTAL.
2 participants
Tested by building the operator image, pushing it to a remote repo and then deploying that remote repo. Confirmed the change by checking the operator logs for the commit id. Once the smbshare was created, I did a kubectl exec into it and checked the generated smb.conf file using the net conf list command
This set of patches contains two changes.