Skip to content
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

proposed fix for #758 - migrating the jenkins-master container startup from install-plugins.sh to jenkins-plugin-cli and migrating /sbin/tini to /usr/bin/tini #764

Conversation

jay-johnson
Copy link

Changes

Proposed changes for getting the jenkins-jenkins pod to start up the jenkins-master container on docker image docker.io/jenkins/jenkins:2.366 and the operator on virtuslab/jenkins-operator:v0.7.1

I documented my notes in the issue:
#758 (comment)

Notes:

  • I only tested this change with minikube 1.24 (I do not have virtualbox installed to test with the make minikube-start per the docs).
  • I see explicit OpenShift if/else blocks that likely need to be reviewed (I do not have access to try the changes on an OpenShift cluster).
  • I did not test the alpine or other jenkins docker images. The proposed path: /usr/bin/tini may not be consistent on other image OS'es.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Reviewer Notes

This changes the jenkins-master container startup. This could impact a lot of users if it is incorrect.

Release Notes

…ins.sh to jenkins-plugin-cli and migrating /sbin/tini to /usr/bin/tini
brokenpip3 added a commit to brokenpip3/kubernetes-operator that referenced this pull request Dec 10, 2022
* the fix was original attempted here:
  jenkinsci#764 but was not
  working correctly due to 2-3 additional changes which needed to be
  done
* removed the openshift check because the env is not mention anywhere
  and also the new jenkins-plugin-cli does not a specific command for
  openshift. Finally this does not make any sense in general, the only
  problem in ocp will be the user id that will be mapped to a random uid
  but that's another story. The command to install the plugins should
  remain the same across different k8s flavours.
@brokenpip3
Copy link
Collaborator

hi @jay-johnson thanks a lot for this PR, I tried your changes but the plugin-install binary change needed some more changes that I made here: #784 based on your first attempt.

Copy link

@khalidjshaikh khalidjshaikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up writing a Dockerfile and copy tini to /sbin/tini. This is a better solution.

prryb pushed a commit that referenced this pull request Jan 12, 2023
…t the newest jenkins lts version (#784)

* fix(seed): fix #742, workaround #698
Original fix proposal: #742 (comment)

* fix(install-plugin.sh): fix #758, #739
* the fix was original attempted here:
  #764 but was not
  working correctly due to 2-3 additional changes which needed to be
  done
* removed the openshift check because the env is not mention anywhere
  and also the new jenkins-plugin-cli does not a specific command for
  openshift. Finally this does not make any sense in general, the only
  problem in ocp will be the user id that will be mapped to a random uid
  but that's another story. The command to install the plugins should
  remain the same across different k8s flavours.

* fix(doc/test): fix /usr/bin/tini in any doc and validation

* fix(jenkins): remove AdminWhitelistRule to avoid jvm stack trace, see: https://www.jenkins.io/doc/book/security/controller-isolation/jep-235/#api-compatibility

* fix(seed): fix seed img built on a previous jvm, fix #761

* fix(plugin): update the base plugin to work with the newest version of
jenkins:lts

* fix(run): fix #778

* fix(backup): add a trap to remove the tmp dir if the tar fail, also fix: #770

* test(chart): update chart values for testing, will revert before merge

* fix(configmap): leftover

* fix(tests): fix seed job test

* fix(e2e)

* fix(e2e): helm

* fix(operator): update the temporary img to reflect latests changes

* Fix Helm e2e tests

* add trap in case of unwanted exit and make shellcheck happy

* chore(plugin): update git ver to 5.0.0

* fix(backup): always force delete the backup directory

* chore(operator): update the temporary img to reflect latest changes

* chore(jenkins): upgrade jenkins latest lts
@brokenpip3
Copy link
Collaborator

hi @jay-johnson this should be completed in #784, if make sense for you you can close the PR :)

@jay-johnson
Copy link
Author

Thanks for fixing this @brokenpip3 + the rest of the team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants