Closed dili91 closed 4 years ago
phlax
commented
4 years ago hi @dili91 not sure why its not working on your system exactly, but keen to figure out. It wfm locally and in CI
I have the same version of docker as you so i dont think that is the problem.
regarding removing -i from the sed command - does this not just prevent it from updating ? certainly that would be the effect on linux
phlax
commented
4 years ago ...also if you test this sandbox with the ./verify.sh locally (and the sed command is working) then you will need to do something like:
git checkout configs/cds.yaml
./verify.shif you want to test it more than once
phlax
commented
4 years ago /assign phlax
dili91
commented
4 years ago Hey @phlax thanks for the prompt reply. Mac comes with a slightly different version of sed compared to GNU's one.
Below command inside verify.sh returns an error and does not replace service1 with service2 on cds.yaml file.
sed -i s/service1/service2/ configs/cds.yaml
I can easily overcome this by either replacing -i with -e or using gsed, that is GNU sed. Earlier I wrote removing, but actually I meant changing the above command to this:
sed -e s/service1/service2/ configs/cds.yaml
That said, I ran the script multiple times as well as testing on my own (manually changing the values) but the updated file is not picked up by the envoy process running on the Docker container.
I'm keen to provide extra troubleshooting details if any
phlax
commented
4 years ago @dili91 so 2 things
im keen to update the verify.sh script to make it work on mac (i have no easy access to any versions atm)
this has always been a bit of a mess on mac, and seems to still be - see https://stackoverflow.com/questions/4247068/sed-command-with-i-option-failing-on-mac-but-works-on-linux
i dont think -e cuts it - it just outputs to the term.
does a variant of this work ?
sed -i'.bak' -e 's/old_link/new_link/g' *if i can get a decent command to work on both ill update the script
can you confirm that after running verify.sh the cds file has changed
phlax
commented
4 years ago if the file is getting updated and the example is still not working then my guess is its something to do with file inodes and envoy not seeing the file as changed (its the only thing i can think of atm)
if this is the case, the first question would be whether the timestamp has changed inside the container
phlax
commented
4 years ago dili91
commented
4 years ago Hey, so:
i dont think -e cuts it - it just outputs to the term.
Yeah sorry, you're right. But afterwards I manually changed the file with vim.
does a variant of this work ?
sed -i'.bak' -e 's/old_link/new_link/g' *
This one applies the desired changes to the file: sed -i'.bak' -e 's/service1/service2/g' configs/cds.yaml. But envoy process in the container does not pick it up
Yes, timestamp is updated:
$ docker exec dynamic-config-fs_proxy_1 ls -alh /var/lib/envoy
total 16K
drwxrwxrwx 5 root root 160 Nov 1 17:22 .
drwxr-xr-x 1 root root 4.0K Nov 1 17:22 ..
-rw-r--r-- 1 root root 396 Nov 1 17:22 cds.yaml
-rw-r--r-- 1 root root 396 Nov 1 17:21 cds.yaml.bak
-rw-r--r-- 1 root root 752 Nov 1 14:36 lds.yamlLast but not least, during my manual changes I used vim. I just found out that vim and inotify seem not to be happy toghether. See this thread as reference.
I solved by updating the file like this, from the host system (my Mac):
docker exec dynamic-config-fs_proxy_1 sed -i s/service1/service2/ /var/lib/envoy/cds.yamlI also tried to replace the original sed command in the verify.sh script with the above command and the verification passed. If interested, I could raise a PR to trigger this update on the container rather than updating the host file. There is a last outstanding issue likely linked to inodes - as you said - but I'm OK with what I found out. I would just mention in the documentation to make sure to use sed or other tools that override the file like sed does.
phlax
commented
4 years ago I solved by updating the file like this, from the host system (my Mac):
great solution!
this also removes the need to give the umask warning in the docs
can you PR to update ?
if not, im happy to
phlax
commented
4 years ago (...just read rest of post...)
yep please! do PR!
phlax
commented
4 years ago on the exec its better to add -T for ci
dili91
commented
4 years ago Sure, will raise a PR then !
I've been struggling a few hours to make this sample work, but no luck. Originally I implemented something very similar on my own: I could see the cds.yaml file properly updated across host and container's filesystem, but envoy did not pick the updated file at runtime.
This is the only update I could spot on proxy logs soon after updating the file:
Finally, I also cloned this repo and ran this project but I got the same results: even though I can see the file properly updated on both host and containers file systems, it seems envoy is not acknowledging any changes.
below verify.sh logs (I had to remove the -i option on sed command in order to use it on my Mac)
Last but not least, my docker setup info, if required:
I tried and noticed the above issues on both envoyproxy/envoy-dev:latest and envoyproxy/envoy:v1.16-latest Docker images.
Thank you. Andrea