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 cloud tracking #383

Merged
merged 5 commits into from
Jul 17, 2023
Merged

Fix cloud tracking #383

merged 5 commits into from
Jul 17, 2023

Conversation

pdk27
Copy link
Collaborator

@pdk27 pdk27 commented Jul 11, 2023

Context

Jenkins Cloud represents user configuration and is bound to the corresponding user input data. i.e. a user-initiated change to the cloud configuration recreates that cloud's instance.

The plugin currently passes the cloud object around to Ec2FleetNode and Ec2FleetNodeComputer in an effort to link them to the cloud they belong to. This becomes problematic when those cloud instances are re-created. The plugin's re-assignment logic has been buggy requiring tracking of oldId etc.

This PR removes the usage of oldId and cloud re-assignments and replaces cloud object tracking with tracking by name making cloud name a special configuration. Hence #382.

Related issues:

Changes

  • [fix] Remove references of oldId and EC2FleetCloudAware class, remove tracking of cloud objects
  • [refactor] Renamed FleetNode#name to instanceId for clarity, added getDescriptor to return sub type
  • [refactor] Added a check to not provision if Jenkins is quieting down and terminating

Supporting changes:

Testing done

  • Cloud config change in UI
  • Cloud config change via Config-as-Code (CasC)
  • Config change during scaling activity / during build / various other scenarios and configurations like maxTotalUses

Now:
(Logs only used for development)

FINE com.amazon.jenkins.ec2fleet.EC2FleetCloud 
*** In EC2FleetCloud constructor for FleetCloud1 obj: com.amazon.jenkins.ec2fleet.EC2FleetCloud@4bfcd4f6

// Config change re-creates EC2FleetCloud object

FINE com.amazon.jenkins.ec2fleet.EC2FleetCloud 
*** In EC2FleetCloud constructor for FleetCloud1 obj: com.amazon.jenkins.ec2fleet.EC2FleetCloud@2d9a84b6

// Printing clouds, nodes, computers and the tracking object 
FINE com.amazon.jenkins.ec2fleet.CloudNanny doRun
In CloudNanny --> getClouds: FleetCloud1 com.amazon.jenkins.ec2fleet.EC2FleetCloud@2d9a84b6

// node's display name --- node object --- cloud name --- cloud object  
FINE com.amazon.jenkins.ec2fleet.CloudNanny doRun
In CloudNanny --> getNodes: FleetCloud1 i-063e03e9aad1fb336---com.amazon.jenkins.ec2fleet.EC2FleetNode[i-063e03e9aad1fb336]---FleetCloud1---cloud: com.amazon.jenkins.ec2fleet.EC2FleetCloud@2d9a84b6

// computer's display name --- computer object --- cloud object 
FINE com.amazon.jenkins.ec2fleet.CloudNanny doRun
In CloudNanny --> getComputers: FleetCloud1 i-063e03e9aad1fb336---com.amazon.jenkins.ec2fleet.EC2FleetNodeComputer@3ad0593e---cloud: com.amazon.jenkins.ec2fleet.EC2FleetCloud@2d9a84b6

Before:

INFO com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils reassign
Trying to reassign Jenkins computer:master
INFO com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils reassign
Trying to reassign Jenkins computer:SpotFleetCloud1 i-01614d523892720da
INFO com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils checkAndReassign
Reassigned com.amazon.jenkins.ec2fleet.EC2FleetNodeComputer@21b05f71 from SpotFleetCloud1 to SpotFleetCloud1
INFO com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils reassign

pdk27 added 4 commits July 11, 2023 15:54
… tracking of cloud instance

[refactor] Added instanceId to FleetNode for clarity, added getDescriptor to return sub type

[refactor] Dont provision if Jenkins is quieting down and terminating

Fix jenkinsci#360
Fix jenkinsci#322
@pdk27 pdk27 added the do not merge Don't merge this (at least not yet) label Jul 11, 2023
@pdk27 pdk27 requested review from naraharip2017 and cjerad July 11, 2023 21:30
@pdk27 pdk27 marked this pull request as ready for review July 11, 2023 22:02
@@ -4,16 +4,12 @@
xmlns:t="/lib/hudson" xmlns:f="/lib/form"
xmlns:c="/lib/credentials">

<f:description>A unique name for this EC2 Fleet cloud</f:description>
<f:description>Warning: Cloud name is used to track agents belonging to this cloud. Please do not modify an existing cloud's name. See <a href="https://github.com/jenkinsci/ec2-fleet-plugin/issues/382">this issue</a> for details</f:description>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this warning the only barrier in place from changing the cloud name? What would happen to existing agents if a cloud name is changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately yes. I meant to create another task to implement disallowing name change. This PR is already handling a lot, so didn't want to overload it.

@@ -338,13 +338,14 @@ public void should_continue_update_after_termination() throws IOException {
j.jenkins.getLabelAtom("momo").nodeProvisioner.suggestReviewNow();
System.out.println("tasks submitted");

// wait full execution
// wait ful l execution
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: spacing on the comment

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