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

Uninstalling a plugin #1531 #1538

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ritikbanger
Copy link
Contributor

@ritikbanger ritikbanger commented Dec 25, 2022

Document removal of a plugin from container

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes

This PR address the following issue: #1531 which is discussed in SIG December 16th Meeting

cc @dduportal @gounthar

Merry Christmas 🎅

@ritikbanger ritikbanger requested a review from a team as a code owner December 25, 2022 08:42
@gounthar
Copy link
Contributor

Merry Christmas too, thanks a lot for your contribution. 🙏

README.md Outdated Show resolved Hide resolved
@ritikbanger
Copy link
Contributor Author

Are we ready to merge or is there any more changes @gounthar @dduportal

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

The install-plugins.sh script is no longer a useful script. The plugins.txt file defines the plugins to be installed, not the plugins to be removed (at least as far as I've used it).

README.md Outdated Show resolved Hide resolved
@MarkEWaite
Copy link
Contributor

From experiments with the jenkins/jenkins:2.375.1 container, I can remove a plugin from the installation by removing the entry from the plugins.txt file, building a new container image, then stopping the existing container and starting with the new container image.

Dockerfile that I used was:

FROM jenkins/jenkins:2.375.1
COPY --chown=jenkins:jenkins plugins.txt $REF/plugins.txt
RUN jenkins-plugin-cli --plugin-file $REF/plugins.txt

However, that simple use case does not cover the cases where the user is preserving their configuration in a separate data volume. In that case, then I believe that additional steps will be required in order to remove the files from $JENKINS_HOME/plugins/* so that the jenkins.sh script that starts the java -jar jenkins.war process is writing the set of plugins from the $REF directory to the plugins directory without having to remove existing files.

@ritikbanger
Copy link
Contributor Author

From experiments with the jenkins/jenkins:2.375.1 container, I can remove a plugin from the installation by removing the entry from the plugins.txt file, building a new container image, then stopping the existing container and starting with the new container image.

Dockerfile that I used was:

FROM jenkins/jenkins:2.375.1
COPY --chown=jenkins:jenkins plugins.txt $REF/plugins.txt
RUN jenkins-plugin-cli --plugin-file $REF/plugins.txt

However, that simple use case does not cover the cases where the user is preserving their configuration in a separate data volume. In that case, then I believe that additional steps will be required in order to remove the files from $JENKINS_HOME/plugins/* so that the jenkins.sh script that starts the java -jar jenkins.war process is writing the set of plugins from the $REF directory to the plugins directory without having to remove existing files.

Added a new commit- b0b6bcf

Let me know, if there are any suggestions.

@ritikbanger ritikbanger requested review from MarkEWaite and removed request for gounthar December 30, 2022 04:12
@ritikbanger
Copy link
Contributor Author

Any update on this

@MarkEWaite
Copy link
Contributor

Any update on this

Sorry for the delay. It will likely be a few more days before I can review.

@gounthar
Copy link
Contributor

gounthar commented Jan 4, 2023

Same for me, sorry.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gounthar gounthar left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution, we do appreciate it. 🙏
Please have a look at my comments and let me know if you agree.
If you don't (which is fine), let's discuss it further.

Once more, thanks.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@MarkEWaite MarkEWaite linked an issue Jan 27, 2023 that may be closed by this pull request
@ritikbanger
Copy link
Contributor Author

@MarkEWaite can you help me with the new updates that I need to add or remove from this PR?

@ritikbanger ritikbanger requested review from gounthar and removed request for MarkEWaite February 5, 2023 14:54
Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Most of this is general documentation that applies to all Jenkinses. Why is it added to the documentation of the Docker images?

@dduportal
Copy link
Contributor

Most of this is general documentation that applies to all Jenkinses. Why is it added to the documentation of the Docker images?

Because it's needed here until a "general documentation with clear examples for Docker" is provided. There are multiple specific elements for Docker image that are required, which does NOT apply to packaged distribution of Jenkins (the fact that plugins should be pre-installed in the image in /usr/share/jenkins/ref but that what Jenkins sees is the data volume in /var/jenkins-home for instance.

@ritikbanger
Copy link
Contributor Author

Hey @dduportal, can you help me with this PR. Its been one month without any update from other maintainers.

@MarkEWaite
Copy link
Contributor

Sorry for the delay @ritikbanger . I'll do my best to help with a review within the next 7 days. @dduportal probably won't be able to help due to other priorities that he needs to handle.

@ritikbanger
Copy link
Contributor Author

@dduportal I have updated the PR with your feedback. Please have a look.

@ritikbanger
Copy link
Contributor Author

@gounthar your review will be highly appreciated

Copy link
Contributor

@gounthar gounthar left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this work. 🤗

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gounthar gounthar left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this work. 🤗

@ritikbanger
Copy link
Contributor Author

@gounthar I think we are ready to merge this.

@gounthar
Copy link
Contributor

gounthar commented Aug 5, 2023

Thank you very much for your contribution and the latest changes, @ritikbanger.
I don't have the authority to merge your pull request, but I have already approved it.

@ritikbanger
Copy link
Contributor Author

@dduportal

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

It's going in a really good direction! I'm still not in agreement with some major elements.

The main point is that it feels like you want to write a step by step guide: the README is not the place for that. I would prefer having a page in jenkins.io's documentation (which requires asking the documentation SIG group to find the perfect place in the table of content).

I've provided feedback to continue this PR and land it by shrinking steps and being more generic (requiring Docker Compose is not acceptable for instance).

Thanks for this work!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dduportal
Copy link
Contributor

Hi @ritikbanger , any news? This PR has been opened for quite some time and it would benefit from an update.

@ritikbanger
Copy link
Contributor Author

I would be updating this PR shortly.

@gounthar
Copy link
Contributor

Thanks. 🤗

@ritikbanger
Copy link
Contributor Author

@dduportal Please have a look.

@dduportal
Copy link
Contributor

@dduportal Please have a look.

Thanks @ritikbanger . I'll check before end of month as I only have low availability for now.

@ritikbanger
Copy link
Contributor Author

@dduportal Please have a look.

Thanks @ritikbanger . I'll check before end of month as I only have low availability for now.

Any update on this

@ritikbanger
Copy link
Contributor Author

Its been 2 years since this PR, if we are not merging it, kindly close it.

@dduportal
Copy link
Contributor

Hi @ritikbanger , many thanks for the content, work and pings. Sorry for not being able to take the required time.

it’s been a long time so i do not remember all the details, but reviewing it again (eg. From scratch) , i feel line this content should mostly be in www.jenkins.io as it is a good step by step which is oriented not only for Docker containers but also jenkins in general. And the readme here should point the user to the www.jenkins.io.
My past proposal to have a first step here seems unwise today as it would mean not spending the right effort.

wdyt @MarkEWaite @gounthar @kmartens27 ?

@gounthar
Copy link
Contributor

gounthar commented Oct 4, 2024

I believe this kind of information would be better suited for the jenkins.io website.

@kmartens27
Copy link

I agree with @gounthar that this would be very helpful information to have on jenkins.io, as uninstalling a plugin is a fairly common practice that would be good to have documented.

@ritikbanger
Copy link
Contributor Author

Could you share me which specific section of this repo should i put this into

@kmartens27
Copy link

Sure thing @ritikbanger! I think the Managing Plugins section of documentation would be the most appropriate. There is already a section for Removing/Uninstalling plugins, so this could be a unique section under that area with it's own "Uninstalling a plugin with Docker" header.

@gounthar
Copy link
Contributor

I agree with @kmartens27 suggestion. A subsection within the existing Uninstalling a Plugin page would indeed be an ideal location for this information.

When adding the new subsection, we should:

  1. Ensure it complements the existing content without duplicating information.
  2. Focus on providing additional, valuable insights specific to the topic at hand.
  3. Maintain consistency with the current structure and style of the documentation.

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.

Missing "Uninstall" plugin section
8 participants