Do not delete configs on a readonly mount #20
Conversation
|
NOTE: the PR is a wip I intend to add more commits to this pr which addresses related issues. |
Following a recent change to the jenkins deployment that made the configmap and the secrets readonly, deleting the configurations that are stored in the config-map resulted in an error message. This patch fixes it by deleting `rm -rf /opt/openshift/configuration` in the `run` script. Fixes: openshiftio/openshift.io#2608 See also: kubernetes/kubernetes#60814
This allow JENKINS_HOME to be setup to a different one when testing the run script locally
Following a recent change to the jenkins deployment that made the configmap and the secrets readonly, deleting the configurations that are stored in the config-map resulted in an error message. This patch fixes it by expanding templates into a temp directory and then copying the expanded templates into JENKINS_HOME Fixes: openshiftio/openshift.io#2608 See also: kubernetes/kubernetes#60814
|
With this patch turns to |
|
Tested the patch in OSIO by running pipeline twice |
|
|
||
| # we don't need the templates | ||
| log_info "Removing template files (*.tpl) from $JENKINS_HOME" | ||
| rm -f "$JENKINS_HOME/"*.tpl |
There was a problem hiding this comment.
Is this remove really necessary? My concerns is that we face the same problem we had some time ago.
There was a problem hiding this comment.
no it shoudn't cause any errors as $JENKINS_HOME is rw and not read-only. If JENKINS_HOME is ro then that is a much bigger problem as nothing can write to it.
There was a problem hiding this comment.
It is not a problem on an error but in performance problems we faced in the past . cc @kbsingh what do you think?
There was a problem hiding this comment.
@lordofthejars Wasn't the root cause for performance issue that we were copying 400MB of plugin which is like close to 300 files?
A reason to do this way using a tmp dir is to ensure that failure of an in between step do not leave the system in a bad state.
There was a problem hiding this comment.
Ok I think we can remove safely because exploring the issue I am mention (openshiftio/openshift.io#2612) I see that it was because of a move not a remove. So go ahead.
| generate_jenkins_config | ||
| generate_credentials | ||
| copy_config_files | ||
| rm -fr "${IMAGE_CONFIG_GEN_DIR}" |
There was a problem hiding this comment.
Same comment as before, I am not sure if it is really good idea to remove all created stuff,
There was a problem hiding this comment.
I am only removing the tmp directory used to expand the templates after they are copied to appropriate locations. So the workflow is like this ...
- check if config needs to be generated
- if so, create a tmp dir and generate all configs there
- copy the expanded configs to appropriate location
- cleanup the tmp dir i.e. rm it
This is all done under $JENKINS_HOME which is required to by read-write mount
There was a problem hiding this comment.
Ok so this remove is just executed at most once, so when an instance is unidled, since everything is already in place then it is not executed right?
There was a problem hiding this comment.
@lordofthejars yes, and no. Given the current config-map no, the config will be generated every time the jenkins boots until we modify existing config-map to add a timestamp see - CONFIG_TIMESTAMP var above and how that is used config_changed method.
There was a problem hiding this comment.
well configs are not so many files IIRC and not so much MB so we are fine.
Following a recent change to the jenkins deployment that made the
configmap and the secrets readonly, deleting the configurations that
are stored in the config-map resulted in an error message.
This patch fixes it by deleting
rm -rf /opt/openshift/configurationin the
runscript.Fixes: openshiftio/openshift.io#2608
See also: kubernetes/kubernetes#60814