-
Notifications
You must be signed in to change notification settings - Fork 61
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
189 code server fixes #204
189 code server fixes #204
Conversation
…e path to the secret file
…de gateway network
…at it doesnt cause errors when code server is not a service
@joshdentremont This probably should have instructions on how to test. I just attempted to run the following after checking out this merge request but I'm seeing an error. Looks like code server is trying to fetch a secret before the file is created/copied from the secrets/template directory. $ make clean
$ make pull Then changed the the
Then ran $ make pull
docker-compose pull
ERROR: Service "code-server" uses an undefined secret "CODE_SERVER_PASSWORD"
make: *** [Makefile:92: pull] Error 1 |
@DonRichards I believe I ran into that before, but I don't think it's related to this pull request. I forget what I did to fix it, but I think adding code server requires make clean, then another make local or make demo to generate the secrets file. |
@joshdentremont It looks like the change you made to the docker-compose.code-server.yml file might be the issue. I ran |
To be frank, I'm not sure why it isn't working with the code you submitted. |
@DonRichards thanks for checking this out. I will try to take a look at this this week and let you know |
I think perhaps the issue is that this is still here? isle-dc/docker-compose.code-server.yml Lines 40 to 41 in 39076d9
|
Doing the following...
...got rid of the
|
Ok, reading through the entire README page again, I realized there might just be a fundamental incompatibility between using "When developing locally, your Drupal site resides in the codebase folder and is bind-mounted into your Drupal container. This lets you update code using the IDE of your choice on your host machine, and the changes are automatically reflected on the Drupal container." Which seems to maybe obviate the need for code-server |
The PR, with lines 40 & 41 of docker-compose.code-server.yml removed, works as expected with |
In Tech Call today, Alan mentioned that So if this PR needs to work with |
@DonRichards @kspurgin I figured out why you were getting that error. You are trying to add code server but not use secrets. In the code server yml it references the secret that is set in the secrets yml file, which your .env is not using. I had moved lines 40 & 41 from docker-compose.code-server.yml to docker-compose.secrets.yml but that made it not work when INCLUDE_CODE_SERVER_SERVICE=false and USE_SECRETS=true We need a way for this to work for all 4 of the following: I suspect this is why it was previously all handled in docker-compose.code-server.yml, which was just hard coded to use secrets even if use secrets was set to false in the .env. I think we probably need to go back to this because secrets.yml can't reference code-server like it can for the other services because it can't guarantee it exists. I will push another commit shortly that addresses the original concern about the path not being correct, but leave the path in code-server.yml. This should get things working, but leaves us open to this problem coming up again if we update the way secrets work and forget about code server since it is handled differently than the rest. |
The behaviour @kspurgin mentioned is what I'm now seeing. I have moved the info for the secret back into the code server yml. This should fix the issue I set out to fix, which was the incorrect path, but not the issue with it not working for make local. I will leave that one to someone more familiar with code server. |
@joshdentremont wrote:
Do we really? If it's worthwhile to handle the secrets paths in one place, could we not just change in sample.env and make clear in the readme section for code-server:
|
Tried with fresh pull of this PR, setting both secrets and code-server to true. make local still failed with the error reported here, which is separate from the
|
Yeah it sounds like there are more issues than I originally realized. I have commented on issue 189 that this PR should fix part of it but not everything. Your idea of noting in the .env that if code server is true then secrets must be as well would work to let us put the path back in the secrets file, if that is something people think is worthwhile. |
The following test worked for me: git checkout joshdentremont/189-code-server
git clean -xfd . # Clean Repository of uncommitted files.
cp sample.env .env
sed -i 's/INCLUDE_CODE_SERVER_SERVICE=false/INCLUDE_CODE_SERVER_SERVICE=true/g' .env # Enable code-server
sed -i 's/TAG=.*/TAG=1.0.0-alpha-10/g' .env # Use latest release
make docker-compose.yml
make generate-secrets
docker-compose up code-server |
Fix for issue #189
Code server yml had the wrong path to the secret file so I updated it to include liv/ in the path.
I also moved the path variable into the secrets.yml file so that it is with the rest of the secret paths in case they need to be changed again in the future.
There was also an issue where code server was listed as a service after traefik in the makefile which caused the gateway network to be overwritten when code server was included as a service. I moved traefik to the end of this list to stop that from happening.
Lastly, I fixed a typo in the makefile where services was spelled serivces