Skip to content

Commit

Permalink
feat: use 'foreground' delete policy to pass the cancel job test (#290)
Browse files Browse the repository at this point in the history
bypass the test case issue, and add more info when error occurs.

Actual reason:
- the default delete policy in k8s has changed to background since v1.20.
- Plus the way you added finalizer is replaceCR, so you will sometimes miss the delete event generated by your client.

---------

Co-authored-by: imbajin <[email protected]>
  • Loading branch information
qwtsc and imbajin authored Dec 4, 2023
1 parent 0c447be commit 9d4d276
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,53 +19,95 @@

import java.util.Objects;

/**
* DefaultJobState is an implementation of the JobState interface.
* It holds the state of a job including the current superstep,
* the maximum superstep, the last superstep statistics, and the job status.
*/
public class DefaultJobState implements JobState {

private int superstep;
private int maxSuperstep;
private SuperstepStat lastSuperstepStat;
private JobStatus jobStatus;

/**
* Sets the current superstep.
* @param superstep The current superstep.
* @return The updated DefaultJobState instance.
*/
public DefaultJobState superstep(int superstep) {
this.superstep = superstep;
return this;
}

/**
* Sets the maximum superstep.
* @param maxSuperstep The maximum superstep.
* @return The updated DefaultJobState instance.
*/
public DefaultJobState maxSuperstep(int maxSuperstep) {
this.maxSuperstep = maxSuperstep;
return this;
}

/**
* Sets the last superstep statistics.
* @param lastSuperstepStat The last superstep statistics.
* @return The updated DefaultJobState instance.
*/
public DefaultJobState lastSuperstepStat(SuperstepStat lastSuperstepStat) {
this.lastSuperstepStat = lastSuperstepStat;
return this;
}

/**
* Sets the job status.
* @param jobStatus The job status.
* @return The updated DefaultJobState instance.
*/
public DefaultJobState jobStatus(JobStatus jobStatus) {
this.jobStatus = jobStatus;
return this;
}

/**
* @return The current superstep.
*/
@Override
public int superstep() {
return this.superstep;
}

/**
* @return The maximum superstep.
*/
@Override
public int maxSuperstep() {
return this.maxSuperstep;
}

/**
* @return The last superstep statistics.
*/
@Override
public SuperstepStat lastSuperstepStat() {
return this.lastSuperstepStat;
}

/**
* @return The job status.
*/
@Override
public JobStatus jobStatus() {
return this.jobStatus;
}

/**
* Checks if the given object is equal to this instance.
* @param o The object to compare with.
* @return true if the given object is equal to this instance, false otherwise.
*/
@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -74,17 +116,30 @@ public boolean equals(Object o) {
if (!(o instanceof DefaultJobState)) {
return false;
}

DefaultJobState jobState = (DefaultJobState) o;
return this.superstep == jobState.superstep &&
this.maxSuperstep == jobState.maxSuperstep &&
Objects.equals(this.lastSuperstepStat,
jobState.lastSuperstepStat) &&
Objects.equals(this.lastSuperstepStat, jobState.lastSuperstepStat) &&
this.jobStatus == jobState.jobStatus;
}

/**
* @return The hash code of this instance.
*/
@Override
public int hashCode() {
return Objects.hash(this.superstep, this.maxSuperstep,
this.lastSuperstepStat, this.jobStatus);
}

/**
* @return A string representation of this instance.
*/
@Override
public String toString() {
return String.format("%s[superstep=%s, maxSuperStep=%s, lastSuperstepStat=%s, " +
"jobStatus=%s]", DefaultJobState.class.getSimpleName(),
superstep, maxSuperstep, lastSuperstepStat, jobStatus);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;

import io.fabric8.kubernetes.api.model.DeletionPropagation;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -282,7 +283,9 @@ private void checkComputerConf(Map<String, String> computerConf,

@Override
public boolean cancelJob(String jobId, Map<String, String> params) {
return this.operation.withName(KubeUtil.crName(jobId)).delete();
return this.operation.withName(KubeUtil.crName(jobId))
.withPropagationPolicy(DeletionPropagation.FOREGROUND)
.delete();
}

@Override
Expand Down

0 comments on commit 9d4d276

Please sign in to comment.