Francisco Borges Aurindo Barros (dd4ad517) at 28 Mar 10:11
fixing tests
Given ongoing audit to Drupal websites, part of the outcome will be lists of websites targeted for deletion.
Prepare 2 scripts:
Update: Users need access to Openshift UI to retrieve webdav password. Perhaps we could block WebDav entirely?
we would also need to tweak the documentation, so advanced access to Openshift UI and customisation would show as deprecated
We need this change to be applied only to new websites, with the possibility to bypass it easily in case there's a need for it - since it is a big change. Therefore, I would propose it's the operator who is responsible from doing this change, based on a label in the namespace. This would allow us to just remove the label in case of need, and as well to add the label to all existing non customised websites if we decide to do so.
Said websites would have no reason for advanced access to the Openshift UI. Therefore both solutions could work. Regarding the first approach, is there any reason why php-fpm would not also mount drupal-modules as read only? Instead of only webdav? To avoid the complexity of keeping 2 separate distros I would try the first approach.
Francisco Borges Aurindo Barros (502a5494) at 27 Mar 11:47
Updating operator to enforce always imagepull policy to all, and up...
... and 3 more commits
Comments for improvement: We do not enforce any policy for
nginx
andphp-fpm
due to the fact S2I will not work with image pull policyIfNotPresent
.
Can you elaborate on this?
Francisco Borges Aurindo Barros (36d2b65f) at 27 Mar 09:18
Francisco Borges Aurindo Barros (bc06b3ef) at 27 Mar 09:18
Merge branch 'make-operator-resilient-to-registry-failure' into 'ma...
... and 1 more commit
With the upcoming Gitlab OTG coming, considerations towards how we handle image pulling were discussed between me and @jhensche .
This MR comes as a proposal of improvement to avoid application downtime due to container registry unavailability, by using a default pulling that will take advantage of the node cached image, and only resort to pull the image when not available in the node.
Part of cern-drupal-distribution#19 Part of https://gitlab.cern.ch/drupal/overview/-/issues/10
Comments for improvement:
We do not enforce any policy for nginx
and php-fpm
due to the fact S2I will not work with image pull policy IfNotPresent
.
Add condition to enforce depending on which case.
Francisco Borges Aurindo Barros (36d2b65f) at 26 Mar 14:19
Tests later, they are currently failing
Francisco Borges Aurindo Barros (6035bc59) at 26 Mar 14:17
Applied suggestions of refactoring and others
... and 33 more commits
I fully agree on naming consistency, but having the const types, I believe it becomes confusing to read the usage case string(InProgress)
, not sure if it clearly states what we expect, maybe if we just have as type "LabelName" ? I would still find it more readable with variable "labelStateCompleted" as variable name.
Wdyt?
The reason behind a generated name is to allow backup re-try without name collision. Same could happen if it is deleted twice a drupalsite with the same name.
Since this is a function, I expect the point where it is called to handle the error we return. We could indeed log it beforehand but then we can end up with duplicated errors.
The goal was to trigger the backup upon deployment deletion (so it is guaranteed to have no actions after), but the drupalsite
resource is kept while the backup is being done.
This being said, the small amount of time difference, we can just work as well watching the change of state of the drupalsite
resource.
Updated to mirror the comment
Nicely spotted, error check added
Agreed