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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
250eca9
Removing a recently added integ test (need some refactoring to make i…
pdk27 Jul 11, 2023
52eb973
Added a warning to cloud name description to prevent users from modif…
pdk27 Jul 11, 2023
ad77186
Deleting files related to cloud instance tracking and reassignments
pdk27 Jul 11, 2023
67c351f
[fix] Remove references of oldId and EC2FleetCloudAware class, remove…
pdk27 Jul 11, 2023
b85ea10
Merge branch 'master' into remove-cloud-tracking-2
pdk27 Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/main/java/com/amazon/jenkins/ec2fleet/AbstractEC2FleetCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,4 @@ protected AbstractEC2FleetCloud(String name) {
public abstract boolean hasExcessCapacity();

public abstract boolean scheduleToTerminate(String instanceId, boolean ignoreMinConstraints, EC2AgentTerminationReason reason);

public abstract String getOldId();

public abstract String getFleet();
}
81 changes: 37 additions & 44 deletions src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.amazon.jenkins.ec2fleet.aws.RegionHelper;
import com.amazon.jenkins.ec2fleet.fleet.EC2Fleet;
import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
import com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils;
import com.amazonaws.services.ec2.AmazonEC2;
import com.amazonaws.services.ec2.model.Instance;
import com.amazonaws.services.ec2.model.InstanceStateName;
Expand All @@ -13,6 +12,7 @@
import hudson.Extension;
import hudson.model.Computer;
import hudson.model.Descriptor;
import hudson.model.Failure;
import hudson.model.Label;
import hudson.model.Node;
import hudson.model.Queue;
Expand Down Expand Up @@ -86,25 +86,6 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud {
private static final Logger LOGGER = Logger.getLogger(EC2FleetCloud.class.getName());
private static final ScheduledExecutorService EXECUTOR = Executors.newSingleThreadScheduledExecutor();

// Counter to keep track of planned nodes per EC2FleetCloud, used in node's display name
private transient AtomicInteger plannedNodeCounter = new AtomicInteger(1);

/**
* Provide unique identifier for this instance of {@link EC2FleetCloud}, <code>transient</code>
* will not be stored. Not available for customer, instead use {@link EC2FleetCloud#name}
* will be used only during Jenkins configuration update <code>config.jelly</code>,
* when new instance of same cloud is created and we need to find old instance and
* repoint resources like {@link Computer} {@link Node} etc.
* <p>
* It's lazy to support old versions which don't have this field at all.
* <p>
* However it's stable, as soon as it will be created and called first uuid will be same
* for all future calls to the same instances of lazy uuid.
*
* @see EC2FleetCloudAware
*/
private transient LazyUuid id;

/**
* Replaced with {@link EC2FleetCloud#awsCredentialsId}
* <p>
Expand Down Expand Up @@ -178,9 +159,11 @@ public class EC2FleetCloud extends AbstractEC2FleetCloud {

private transient ArrayList<ScheduledFuture<?>> plannedNodeScheduledFutures;

// Counter to keep track of planned nodes per EC2FleetCloud, used in node's display name
private transient AtomicInteger plannedNodeCounter = new AtomicInteger(1);

@DataBoundConstructor
public EC2FleetCloud(final String name,
final String oldId,
final String awsCredentialsId,
final @Deprecated String credentialsId,
final String region,
Expand Down Expand Up @@ -238,8 +221,6 @@ public EC2FleetCloud(final String name,
if (fleet != null) {
this.stats = EC2Fleets.get(fleet).getState(
getAwsCredentialsId(), region, endpoint, getFleet());
// Reassign existing nodes/computer with new reference of cloud
EC2FleetCloudAwareUtils.reassign(fleet, this);
}
}

Expand All @@ -259,16 +240,6 @@ public String getAwsCredentialsId() {
return StringUtils.isNotBlank(awsCredentialsId) ? awsCredentialsId : credentialsId;
}

/**
* Called old as will be used by new instance of cloud, for
* which this id is old (not current)
*
* @return id of current cloud
*/
public String getOldId() {
return id.getValue();
}

public boolean isDisableTaskResubmit() {
return disableTaskResubmit;
}
Expand Down Expand Up @@ -298,7 +269,6 @@ public String getEndpoint() {
return endpoint;
}

@Override
public String getFleet() {
return fleet;
}
Expand Down Expand Up @@ -419,6 +389,15 @@ private synchronized int getNextPlannedNodeCounter() {

@Override
public synchronized Collection<NodeProvisioner.PlannedNode> provision(@Nonnull final Cloud.CloudState cloudState, final int excessWorkload) {
Jenkins jenkinsInstance = Jenkins.get();
if (jenkinsInstance.isQuietingDown()) {
LOGGER.log(Level.FINE, "Not provisioning nodes, Jenkins instance is quieting down");
return Collections.emptyList();
} else if (jenkinsInstance.isTerminating()) {
LOGGER.log(Level.FINE, "Not provisioning nodes, Jenkins instance is terminating");
return Collections.emptyList();
}

fine("excessWorkload %s", excessWorkload);

if (stats == null) {
Expand Down Expand Up @@ -469,6 +448,7 @@ public synchronized Collection<NodeProvisioner.PlannedNode> provision(@Nonnull f
// This protects us from leaving planned nodes stranded within Jenkins NodeProvisioner when the Fleet
// is updated or removed before it can scale. After scaling, EC2FleetOnlineChecker will cancel the future
// if something happens to the Fleet.
// TODO: refactor to consolidate logic with EC2FleetOnlineChecker
final ScheduledFuture<?> scheduledFuture = EXECUTOR.schedule(() -> {
if (completableFuture.isDone()) {
return;
Expand Down Expand Up @@ -644,7 +624,7 @@ public void run() {

final Set<String> jenkinsInstances = new HashSet<>();
for (final Node node : jenkins.getNodes()) {
if (node instanceof EC2FleetNode && ((EC2FleetNode) node).getCloud().getFleet().equals(fleet)) {
if (node instanceof EC2FleetNode && ((EC2FleetCloud)((EC2FleetNode) node).getCloud()).getFleet().equals(fleet)) {
jenkinsInstances.add(node.getNodeName());
}
}
Expand Down Expand Up @@ -776,7 +756,6 @@ public synchronized boolean scheduleToTerminate(final String instanceId, final b
@Override
public boolean canProvision(final Cloud.CloudState cloudState) {
final Label label = cloudState.getLabel();
fine("CanProvision called on fleet: \"" + this.labelString + "\" wanting: \"" + (label == null ? "(unspecified)" : label.getName()) + "\".");
if (fleet == null) {
fine("Fleet/ASG for cloud is null, returning false");
return false;
Expand All @@ -786,7 +765,7 @@ public boolean canProvision(final Cloud.CloudState cloudState) {
return false;
}
if (label != null && !Label.parse(this.labelString).containsAll(label.listAtoms())) {
fine("Label '%s' not found within Fleet's labels '%s', returning false", label, this.labelString);
finer("Label '%s' not found within Fleet's labels '%s', returning false", label, this.labelString);
return false;
}
return true;
Expand All @@ -798,8 +777,6 @@ private Object readResolve() {
}

private void init() {
id = new LazyUuid();

plannedNodesCache = new HashSet<>();
instanceIdsToTerminate = new HashMap<>();
plannedNodeScheduledFutures = new ArrayList<>();
Expand Down Expand Up @@ -864,7 +841,7 @@ private void addNewAgent(final AmazonEC2 ec2, final Instance instance, FleetStat
final Node.Mode nodeMode = restrictUsage ? Node.Mode.EXCLUSIVE : Node.Mode.NORMAL;
final EC2FleetNode node = new EC2FleetNode(instanceId, "Fleet agent for " + instanceId,
effectiveFsRoot, effectiveNumExecutors, nodeMode, labelString, new ArrayList<NodeProperty<?>>(),
this, computerLauncher, maxTotalUses);
this.name, computerLauncher, maxTotalUses);

// Initialize our retention strategy
node.setRetentionStrategy(new EC2RetentionStrategy());
Expand Down Expand Up @@ -905,7 +882,7 @@ private int getCurrentSpareInstanceCount(final FleetStateStats currentState, fin
}

// Do not count computer if it is not a part of the given fleet
if (!Objects.equals(((EC2FleetNodeComputer) computer).getCloud().getFleet(), currentState.getFleetId())) {
if (!Objects.equals(((EC2FleetCloud)((EC2FleetNodeComputer) computer).getCloud()).getFleet(), currentState.getFleetId())) {
continue;
}
currentBusyInstances++;
Expand All @@ -928,6 +905,10 @@ private void fine(final String msg, final Object... args) {
LOGGER.fine(getLogPrefix() + String.format(msg, args));
}

private void finer(final String msg, final Object... args) {
LOGGER.finer(getLogPrefix() + String.format(msg, args));
}

private void warning(final String msg, final Object... args) {
LOGGER.warning(getLogPrefix() + String.format(msg, args));
}
Expand All @@ -936,6 +917,11 @@ private void warning(final Throwable t, final String msg, final Object... args)
LOGGER.log(Level.WARNING, getLogPrefix() + String.format(msg, args), t);
}

@Override
public DescriptorImpl getDescriptor() {
return (DescriptorImpl) super.getDescriptor();
}

@Extension
@SuppressWarnings("unused")
public static class DescriptorImpl extends Descriptor<Cloud> {
Expand Down Expand Up @@ -972,6 +958,15 @@ public FormValidation doCheckMaxTotalUses(@QueryParameter String value) {
return FormValidation.error("Maximum Total Uses must be greater or equal to -1");
}

public FormValidation doCheckCloudName(@QueryParameter final String name) {
try {
Jenkins.checkGoodName(name);
} catch (Failure e) {
return FormValidation.error(e.getMessage());
}
return FormValidation.ok();
}

public ListBoxModel doFillFleetItems(@QueryParameter final boolean showAllFleets,
@QueryParameter final String region,
@QueryParameter final String endpoint,
Expand All @@ -985,7 +980,7 @@ public ListBoxModel doFillFleetItems(@QueryParameter final boolean showAllFleets
awsCredentialsId, region, endpoint, model, fleet, showAllFleets);
}
} catch (final Exception ex) {
LOGGER.log(Level.WARNING, String.format("Cannot describe fleets in %s or by endpoint %s", region, endpoint), ex);
LOGGER.log(Level.WARNING, String.format("Cannot describe fleets in '%s' or by endpoint '%s'", region, endpoint), ex);
return model;
}

Expand Down Expand Up @@ -1026,7 +1021,5 @@ public boolean configure(final StaplerRequest req, final JSONObject formData) th
save();
return super.configure(req, formData);
}

}

}
23 changes: 0 additions & 23 deletions src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloudAware.java
View file
Open in desktop

This file was deleted.

58 changes: 15 additions & 43 deletions src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.amazon.jenkins.ec2fleet.aws.RegionHelper;
import com.amazon.jenkins.ec2fleet.fleet.EC2Fleets;
import com.amazon.jenkins.ec2fleet.fleet.EC2SpotFleet;
import com.amazon.jenkins.ec2fleet.utils.EC2FleetCloudAwareUtils;
import com.amazonaws.services.cloudformation.AmazonCloudFormation;
import com.amazonaws.services.cloudformation.model.StackStatus;
import com.amazonaws.services.ec2.AmazonEC2;
Expand All @@ -16,7 +15,6 @@
import com.cloudbees.jenkins.plugins.awscredentials.AWSCredentialsHelper;
import hudson.Extension;
import hudson.model.AbstractProject;
import hudson.model.Computer;
import hudson.model.Descriptor;
import hudson.model.Item;
import hudson.model.Label;
Expand Down Expand Up @@ -75,22 +73,6 @@ public class EC2FleetLabelCloud extends AbstractEC2FleetCloud {
private static final SimpleFormatter sf = new SimpleFormatter();
private static final Logger LOGGER = Logger.getLogger(EC2FleetLabelCloud.class.getName());

/**
* Provide unique identifier for this instance of {@link EC2FleetLabelCloud}, <code>transient</code>
* will not be stored. Not available for customer, instead use {@link EC2FleetLabelCloud#name}
* will be used only during Jenkins configuration update <code>config.jelly</code>,
* when new instance of same cloud is created and we need to find old instance and
* repoint resources like {@link Computer} {@link Node} etc.
* <p>
* It's lazy to support old versions which don't have this field at all.
* <p>
* However it's stable, as soon as it will be created and called first uuid will be same
* for all future calls to the same instances of lazy uuid.
*
* @see EC2FleetCloudAware
*/
private transient LazyUuid id;

private final String awsCredentialsId;
private final String region;
private final String endpoint;
Expand Down Expand Up @@ -123,7 +105,6 @@ public class EC2FleetLabelCloud extends AbstractEC2FleetCloud {

@DataBoundConstructor
public EC2FleetLabelCloud(final String name,
final String oldId,
final String awsCredentialsId,
final String region,
final String endpoint,
Expand Down Expand Up @@ -162,12 +143,6 @@ public EC2FleetLabelCloud(final String name,
this.cloudStatusIntervalSec = cloudStatusIntervalSec;
this.noDelayProvision = noDelayProvision;
this.ec2KeyPairName = ec2KeyPairName;

if (StringUtils.isNotEmpty(oldId)) {
// existent cloud was modified, let's re-assign all dependencies of old cloud instance
// to new one
EC2FleetCloudAwareUtils.reassign(oldId, this);
}
}

public String getEc2KeyPairName() {
Expand All @@ -182,22 +157,6 @@ public String getAwsCredentialsId() {
return awsCredentialsId;
}

/**
* Called old as will be used by new instance of cloud, for
* which this id is old (not current)
*
* @return id of current cloud
*/
public String getOldId() {
return id.getValue();
}

@Override
public String getFleet() {
// TODO: We need a way to map existing node/computer's cloud reference rather than relying on oldId
return null;
}

public boolean isDisableTaskResubmit() {
return disableTaskResubmit;
}
Expand Down Expand Up @@ -295,6 +254,15 @@ public synchronized boolean hasExcessCapacity() {

@Override
public synchronized Collection<NodeProvisioner.PlannedNode> provision(@Nonnull final Cloud.CloudState cloudState, int excessWorkload) {
Jenkins jenkinsInstance = Jenkins.get();
if (jenkinsInstance.isQuietingDown()) {
LOGGER.log(Level.FINE, "Not provisioning nodes, Jenkins instance is quieting down");
return Collections.emptyList();
} else if (jenkinsInstance.isTerminating()) {
LOGGER.log(Level.FINE, "Not provisioning nodes, Jenkins instance is terminating");
return Collections.emptyList();
}

info("excessWorkload %s", excessWorkload);

final Label label = cloudState.getLabel();
Expand Down Expand Up @@ -608,7 +576,6 @@ private Object readResolve() {
}

private void init() {
id = new LazyUuid();
states = new HashMap<>();
}

Expand Down Expand Up @@ -653,7 +620,7 @@ private void addNewAgent(
//TODO: Add maxTotalUses to EC2FleetLabelCloud similar to EC2FleetCloud
final EC2FleetNode node = new EC2FleetNode(instanceId, "Fleet agent for " + instanceId,
effectiveFsRoot, effectiveNumExecutors, nodeMode, labelString, new ArrayList<NodeProperty<?>>(),
this, computerLauncher, -1);
this.name, computerLauncher, -1);

// Initialize our retention strategy
node.setRetentionStrategy(new EC2RetentionStrategy());
Expand Down Expand Up @@ -812,6 +779,11 @@ public void run() {
}
}

@Override
public EC2FleetLabelCloud.DescriptorImpl getDescriptor() {
return (EC2FleetLabelCloud.DescriptorImpl) super.getDescriptor();
}

@Extension
@SuppressWarnings("unused")
public static class DescriptorImpl extends Descriptor<Cloud> {
Expand Down
Loading