Autodeploy unable to git pull the newer env.local file.#597
Conversation
This assume the env.local file is part of BIRDHOUSE_AUTODEPLOY_EXTRA_REPOS. Previously, the env.local file could be a symlink to anywhere. Now it can only be a symlink to a file under one of the folders listed in BIRDHOUSE_AUTODEPLOY_EXTRA_REPOS. But the newer docker image seems to not allow both volume-mounting the parent dir and the file at the same time. Can only have one of the two.
github-actions
bot
added
documentation
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/3738/Result ❌ FAILUREBIRDHOUSE_DEPLOY_BRANCH : fix-autodeploy-not-pulling-env.local DACCS_IAC_BRANCH : master DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-91.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/485/NOTEBOOK TEST RESULTS |
fmigneault
left a comment
fmigneault
left a comment
There was a problem hiding this comment.
Looks good to me, but I am not a user of autodeploy, so maybe better to double check with @mishaschwartz
|
@mishaschwartz feel free to comment again on this PR and if anything we can open a follow up PR to address your concerns. However I will have to merge this PR now because this is blocking Ouranos from catching up with tip of this repo in production. |
|
I also don't use autodeploy but I have a few questions:
Unless I'm missing something I've very confused about this fix. |
Autodeploy is a 2 stages process. First stage is to check whether any repos need to be updated. This needs to read Second stage is actually performing the autodeploy (stop the full stack, git pull all the repos, restart the full stack). This second stage is skipped if no repo needs update. If this stage is actually triggered, to stop/start it uses
We never had the need because when volume-mounting a symlink, it actually follow the link and volume-mount the real path already. The not writable error I got is actually for the real path.
Because the repo that contain Recall this is the recommanded layout if birdhouse-deploy/birdhouse/README.rst Lines 118 to 131 in 3e00782 And we suggest in birdhouse-deploy/birdhouse/env.local.example Line 142 in 3e00782 And in the autodeploy scheduler job we volume-mount all repos in the var So basically we were double volume-mounting the folder containing
Yes it took me a while to get this too because I could not understand why this suddenly do not work. I rebooted my VM twice thinking something got stuck on my VM. Then I had to reproduce on another VM to be sure it's not my first VM that suddenly gone mad. |
|
OK I understand how this specifically fixes things for PAVICS now but this change will still break things for anyone else that is using a more conventional structure where No one else as far as I know is using the autodeploy but if the code for it is in this repo then I don't want to force people to use a very specific PAVICS-oriented setup to run it. There is no need for I'm also concerned that you're putting your I think it's a major security issue to encourage users (in the documentation) to put their local environment file under source control at all. Here is what I would suggest.... this should work for both your setup and anyone else's who is not symlinking In your case, when the Since that file is mounted under |
The current checkout of birdhouse-compose is not in
I thought allowing
Exactly, this 100% private is on our local source control, not on the internet. Are you suggesting we should add a reminder next to our recommendation to source-control
If
This is a good suggestion, I can give it a try. Hopefully it solves our nested mount-points problem and not create new problems because we are overriding |
That's only true if it is in the
The environment variable is meant to be used internally yes, but that variable is set when you run
Nice!
Yeah I think that's a good idea. I think that there's a very good chance that people automatically think that if it's a git repository then it's hosted by github/gitlab/bitbucket etc.
Daily backups with the backup scheduler job!!
True, but in my case I could get most of that with backups and scheduled manual updates. I see the advantages you describe I just also see the risks if you don't secure the credentials in env.local properly. It's always a trade-off.
It shouldn't because |
Sorry I meant when the
Oh ! I didn't register that new option The symlink is actually the way to "persist" this override value.
I am not talking about And there is the question of persistence of the override of Obviously we can not persist How can this new value of |
I really recommend you start using bin/birdhouse. We made that script to be the interface for the stack. If you're using something else things may change for you when the stack updates. But that is unrelated to the current discussion...
Well there are a few usual strategies for this...
Note that the scheduler job is essentially doing strategy 2. It notes the location of the local env file when the stack is first started up and saves that location (by writing a template file) for the scheduler job to use.
I think a lot of your issues could be solved if you used this environment variable and put it in your However, the strategies that I mentioned above also solve the "persistence" problem that you mentioned. |
Eventually yes. I have many external scripts and components that uses That's pretty low on my list. Right now I want to go to prod with tip of BH and there is #593 and #598 blocking me. Then after that I want all the new metrics and longterm metrics storage to work.
The scheduler job can save the location of the local env file when the stack is first started up because it has a job to save to ! Not all scripts have this availability (no jobs associated) or does this explicitly in their job. So option 2 is not a 100% foolproof, option 3 is. Once the symlink is there, it is completely transparent to any scripts or jobs authors.
Setting So I guess we can use |
|
I get what you're saying but I think we're getting a bit side tracked here. My point is just this: Your set-up is a good, valid use-case. I'm not asking you to change how you have things set up. Please see if the change I suggested also works for you so that we can support your use-case as well as others. Here is my suggestion again: |
Yes I will for sure try what you suggest. If being able to support both methods (the symlink and What I am trying to say is |
That is a choice that PAVICS can make but should not be dictated to all users. Your use-case is not the only valid use-case. |
…IRDHOUSE_LOCAL_ENV via env var and not symlink See #597 (comment).
Woo, if I was dictating my choice, I would have remove that variable ! I think you are mis-understanding here. I am simply saying between the 2 approaches, the symlink one is more transparent and compatible with wider range of external scripts and jobs. |
…loy raven and xclim testdata missing logs (#601) ## Overview ### Changes - scheduler-job-autodeploy: make compatible with change of `BIRDHOUSE_LOCAL_ENV` via env var and not symlink See #597 (comment). ### Fixes - scheduler-jobs: fix deploy raven and xclim testdata missing logs, forgot delayed eval ## CI Operations <!-- The test suite can be run using a different DACCS config with ``birdhouse_daccs_configs_branch: branch_name`` in the PR description. To globally skip the test suite regardless of the commit message use ``birdhouse_skip_ci`` set to ``true`` in the PR description. Using ``[<cmd>]`` (with the brackets) where ``<cmd> = skip ci`` in the commit message will override ``birdhouse_skip_ci`` from the PR description. Such commit command can be used to override the PR description behavior for a specific commit update. However, a commit message cannot 'force run' a PR which the description turns off the CI. To run the CI, the PR should instead be updated with a ``true`` value, and a running message can be posted in following PR comments to trigger tests once again. --> birdhouse_daccs_configs_branch: master birdhouse_skip_ci: false
…as env var again" This reverts commit a1f649f. Have error 'error: unable to unlink old 'env.local': Resource busy' in autodeploy.log. Even if `env.local` is not physically in `COMPOSE_DIR`, its parent dir is still in the list of `BIRDHOUSE_AUTODEPLOY_EXTRA_REPOS` so we again have the same problem describe in #597 (comment) where double volume-mounting the folder containing `env.local` and `env.local` itself is not allowed for write-access (readonly-access is fine) since the newer Docker image.
4 participants
Overview
Autodeploy unable to git pull the newer
env.localfile.Previously the
env.localfile could be a symlink to a file anywhere on thedisk.
Now
env.localcan only be a symlink to a file under a folder listed inBIRDHOUSE_AUTODEPLOY_EXTRA_REPOS.This is a new restriction from the newer autodeploy docker image in the
previous release
2.18.5below. We can not volume-mount both the parent dirand the file at the same time and have write access. We can only
volume-mount one of the two. For read-only access, it is still okay to
volume-mount both.
Changes
Non-breaking changes
Breaking changes
Related Issue / Discussion
CI Operations
birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false