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

Remove cloudId and use the name entered by the user directly. #827

Merged
merged 1 commit into from
Apr 2, 2023

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Mar 31, 2023

This makes the plugin compatible with cloud copy feature that is being introduced in Jenkins core, cf. jenkinsci/jenkins#7658

  • 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
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

This makes the plugin compatible with cloud copy feature that is being
introduced in Jenkins core
@daniel-beck
Copy link
Member

Originally added for JENKINS-15081 in 9095d31. Did you check that this doesn't reopen (from a Jira comment) this issue?

Basically what I found is previously the plugin was typically retrieving the first EC2Cloud instead of the EC2Cloud that the specific slave belongs to. This is what I've addressed in my changes. I tested it and confirmed with two different EC2Clouds added and two templates (one from each cloud), that I could properly add and delete instances from each template, which I previously couldn't do with any templates from the second cloud.

(Might be specific to the current config UI and obsolete with your core change, but in case anyone would use old core/new plugin.)

@Vlatombe
Copy link
Member Author

JENKINS-15081 seems to predate the ability to provide a custom name for the EC2Cloud, which was the problem in their case (the cloud name was only computed based on the region, so if you had 2 EC2Cloud instances targetting the same region, they would mix up because of using the same name)

@timja timja requested a review from a team April 2, 2023 13:02
@res0nance res0nance merged commit 2965288 into jenkinsci:master Apr 2, 2023
@mwebber
Copy link

mwebber commented Apr 4, 2023

Reported as JENKINS-70987

@Vlatombe , I am running Jenkins 2.387.1 (latest LTS). When I upgraded the ec2-plugin from 2.0.6 to 2.0.7, I found that all Agents terminated after 10 minutes, which is our Idle termination time, even if a job was running on the Agent. In all cases, Agents were terminated 10 minutes after starting.

When we reverted back to 2.0.6, the problem went away, so this looks like a bug introduced in 2.0.7.
There were 2 PRs in that release, this one #827, and #825.

The Jenkins job log shows this:

Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2AbstractSlave idleTimeout
EC2 instance idle time expired: i-0a844c202e5d3ac3d
Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2AbstractSlave stop
EC2 instance stop request sent for i-0a844c202e5d3ac3d
Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2RetentionStrategy internalCheck

@mwebber mwebber mentioned this pull request Apr 4, 2023
6 tasks
@res0nance
Copy link
Contributor

Reported as JENKINS-70987

@Vlatombe , I am running Jenkins 2.387.1 (latest LTS). When I upgraded the ec2-plugin from 2.0.6 to 2.0.7, I found that all Agents terminated after 10 minutes, which is our Idle termination time, even if a job was running on the Agent. In all cases, Agents were terminated 10 minutes after starting.

When we reverted back to 2.0.6, the problem went away, so this looks like a bug introduced in 2.0.7. There were 2 PRs in that release, this one #827, and #825.

The Jenkins job log shows this:

Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2AbstractSlave idleTimeout
EC2 instance idle time expired: i-0a844c202e5d3ac3d
Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2AbstractSlave stop
EC2 instance stop request sent for i-0a844c202e5d3ac3d
Apr 04, 2023 3:24:40 PM INFO hudson.plugins.ec2.EC2RetentionStrategy internalCheck

I'm guessing this is the PR that causes the issue. After an upgrade does removing and re-adding the cloud resolve the issue?

@mwebber
Copy link

mwebber commented May 4, 2023

@res0nance

After an upgrade does removing and re-adding the cloud resolve the issue?

Yes, that did fix the problem. Specifically, I did the following:

  • Delete all Agent instances
  • Delete all Cloud definitions
  • Update the ec2-plugin from 2.0.6 to 2.0.7 (and restart controller)
  • Recreate the Cloud and Agent definitions

basil added a commit to jenkinsci/configuration-as-code-plugin that referenced this pull request Sep 13, 2023
@basil
Copy link
Member

basil commented Sep 13, 2023

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertNotNull(Assert.java:713)
	at org.junit.Assert.assertNotNull(Assert.java:723)
	at io.jenkins.plugins.casc.EC2CloudTest.configure_ec2_cloud(EC2CloudTest.java:35)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:607)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.lang.Thread.run(Thread.java:833)

renovate bot pushed a commit to jenkinsci/configuration-as-code-plugin that referenced this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants