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

fix(operator): Attempt to fix all the major issues present atm against the newest jenkins lts version #784

Merged
merged 21 commits into from
Jan 12, 2023

Conversation

brokenpip3
Copy link
Collaborator

@brokenpip3 brokenpip3 commented Dec 10, 2022

Changes

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.

Release Notes

This project needs love ❤️

I tried to be part of it by fixing and testing all the changes that the operator needs to run and work correctly against the newest versions of jenkins (2.375.1) and the jenkins inbound agent (4.10-3 atm).
Most of the changes were suggested by the community, I'm doing this hoping that fixing all the important issues will trigger the maintainers love and a new version of the operator/helm chart.

With these changes all the functionalities (jenkins, configuration, seed agent, backup) seems working correctly, happy to receive any feedback and happy to discuss any change I made. If you want to try this in your local env you can use my temporary img: quay.io/brokenpip3/jenkins-kubernetes-operator:f312d3d4

Tagging virtuslab maintainers @thegolabek @prryb @Sig00rd

* 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 Author

Summarizing 2 Failures:

[Fail] Jenkins controller when restarting Jenkins master pod [It] new Jenkins Master pod should be created 
/home/runner/work/kubernetes-operator/kubernetes-operator/test/e2e/jenkins_test.go:247

[Fail] Jenkins controller when running Jenkins safe restart [It] authorization strategy is not overwritten 
/home/runner/work/kubernetes-operator/kubernetes-operator/test/e2e/wait.go:96

Ran 6 of 6 Specs in 1880.778 seconds
FAIL! -- 4 Passed | 2 Failed | 0 Pending | 0 Skipped

I tried yesterday to fix the authorization strategy is not overwritten but I was not able to understand why is failing, it's seems that the configmap that create here:

func configureAuthorizationToUnSecure(namespace, configMapName string) {
is not respected, will try to debug it more. Any hints?

also the new Jenkins Master pod should be created is working under my repo (same code), so I need to understand why is not working here.

@toabi
Copy link

toabi commented Dec 12, 2022

I tried yesterday to fix the authorization strategy is not overwritten

Hmm. I don't know much about this, but running the groovy in my running jenkins-operator jenkins instance. The test case would also fail there, so I'm wondering what this test is for.

image

@brokenpip3
Copy link
Collaborator Author

Hi @toabi thanks a lot for helping! 🙌

The fact that in your instance is raising the error is totally correct because that test is trying do:

import hudson.security.*
def jenkins = jenkins.model.Jenkins.getInstance()
def strategy = new AuthorizationStrategy.Unsecured()
jenkins.setAuthorizationStrategy(strategy)
jenkins.save()
  • then restart the jenkins pod and check if the settings is still present by running that groovy script.

2 motivations behind the failure that comes to my mind:

  • the operator does not read correctly the config in the set-unsecured-authorization.groovy script in the configmap
  • In the versions of jenkins jenkins itself is removing that not-secure setting by default

What we can do to test it is trying to replicate manually the configmap creation and the jenkins restart

@brokenpip3
Copy link
Collaborator Author

yesterday looking at the code I discovered that authorization strategy is not overwritten is not the specific step that is failing but instead the restart after that check is the one:

STEP: checking if Authorization Strategy Unsecured is set
STEP: waiting for Jenkins safe restart
Safe restart status: 503, err: %!s(<nil>)
Safe restart status: 503, err: %!s(<nil>)
Safe restart status: 503, err: %!s(<nil>)
{"level":"info","ts":1670845823.2374532,"logger":"controller-jenkins","msg":"Container 'jenkins-master' is terminated, status '{Name:jenkins-master State:{Waiting:nil Running:nil Terminated:&ContainerStateTerminated{ExitCode:5,Signal:0,Reason:Error,Message:,StartedAt:2022-12-12 11:47:59 +0000 UTC,FinishedAt:2022-12-12 11:50:22 +0000 UTC,ContainerID:docker://34129b0150c6fa46e7ca055140703dd68287a2ab8fb4f045c44b8d41b5f3765d,}} LastTerminationState:{Waiting:nil Running:nil Terminated:nil} Ready:false RestartCount:0 Image:jenkins/jenkins:2.375.1-lts ImageID:docker-pullable://jenkins/jenkins@sha256:23b8713004846412ad7cd72ed2debe49f3dc73d70ea1436c44a8cfa202fb405f ContainerID:docker://34129b0150c6fa46e7ca055140703dd68287a2ab8fb4f045c44b8d41b5f3765d Started:0xc000f3845e}'","cr":"e2e"}

My jenkins instance is able to run a safeRestart and a restart without any problem, I need to dig more will do during these days

@brokenpip3
Copy link
Collaborator Author

brokenpip3 commented Dec 21, 2022

I'm start thinking that test is broken even vs master, I tried to spin up the test in my fork with only a space added in README and it's still failing (even more).

Any hints here? @prryb @tomaszsek?
With this PR I have tried to fix as many issues as possible especially the most important one which will allow the operator to use the newest versions of the Jenkins LTS image. Please help me and the community of users of the operator by reviewing my PR and helping me with the failing test.
thanks

@prryb
Copy link
Collaborator

prryb commented Jan 8, 2023

Hey, thanks for contributing. This project is a bit in limbo at the moment, the team has moved on. I'm working on figuring out its future.

Nevertheless, I should find some time this week to look at this PR and run some tests. 👍

@prryb
Copy link
Collaborator

prryb commented Jan 8, 2023

It looks like it's complaining because git plugin is in the old version 4.14.3. Could you bump it up to 5.0.0?

@brokenpip3
Copy link
Collaborator Author

Hey, thanks for contributing. This project is a bit in limbo at the moment, the team has moved on. I'm working on figuring out its future.

Nevertheless, I should find some time this week to look at this PR and run some tests. +1

This is a great news! thanks for taking time to check the PR and figure out the future of this project.
If you need maintainer in your team I will be happy to be part of that, I use the operator a lot both at personal and professional level and my trying to learn golang and how to write k8s operator and this could be a great project to start with. :)

It looks like it's complaining because git plugin is in the old version 4.14.3. Could you bump it up to 5.0.0?

Yeah saw that 2 days ago, let me fix it right now ^_^

@brokenpip3
Copy link
Collaborator Author

@prryb like you see it's always authorization strategy is not overwritten the one that is failing but like I mention here #784 (comment) it's the "safe restart" the real issue. I tried to study/learn how the test library ginkgo works but I was not able to understand the issue, in all the tests I made manually the instance is able to safe restart.

@Harguer
Copy link

Harguer commented Jan 9, 2023

hey @brokenpip3 , @prryb do you guys know when these changes will be available soon in the dockerhub repo? Today was a new kubernetes_client_api plugin version released and it is breaking communication with jenkins k8s agents, to fix it, it needs new jenkins lts and seems you guys are fixing the issue here 765. So I was wondering when your changes will be available on that dockerhub repo. I think in the meantime i can create my own image and put it in my local.

@prryb
Copy link
Collaborator

prryb commented Jan 10, 2023

So 2 tests timed out, but looking at the logs, after Jenkins restarts, the configuration is applied anyway. Those tests have always been a bit flakey, and quite lengthy.

@prryb
Copy link
Collaborator

prryb commented Jan 10, 2023

Hey @Harguer, once that that PR is merged, a snapshot build will be published, so you'll be able to verify if all works correctly after those fixes. As for when: I'm aiming for "sometime this week".

@brokenpip3
Copy link
Collaborator Author

A new lts version of jenkins is out, I already tested the upgrade and the new base plugins, will push the changes soon

@chickenkiller
Copy link

Great I'm impatient to see that new release! Thanks a lot for the great work.

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.

None yet

5 participants