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

Integrated code lifecycle: Allow admins to set build timeout options via application properties #9603

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class BuildJobManagementService {

private final ReentrantLock lock = new ReentrantLock();

@Value("${artemis.continuous-integration.timeout-seconds:120}")
@Value("${artemis.continuous-integration.build-timeout-seconds.max:240}")
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
private int timeoutSeconds;

@Value("${artemis.continuous-integration.asynchronous:true}")
Expand Down Expand Up @@ -152,7 +152,7 @@ public CompletableFuture<BuildResult> executeBuildJob(BuildJobQueueItem buildJob
}

int buildJobTimeoutSeconds;
if (buildJobItem.buildConfig().timeoutSeconds() != 0 && buildJobItem.buildConfig().timeoutSeconds() < this.timeoutSeconds) {
if (buildJobItem.buildConfig().timeoutSeconds() > 0 && buildJobItem.buildConfig().timeoutSeconds() < this.timeoutSeconds) {
buildJobTimeoutSeconds = buildJobItem.buildConfig().timeoutSeconds();
}
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,12 @@ public final class Constants {

public static final String CONTINUOUS_INTEGRATION_NAME = "continuousIntegrationName";

public static final String INSTRUCTOR_BUILD_TIMEOUT_MIN_OPTION = "buildTimeoutMin";

public static final String INSTRUCTOR_BUILD_TIMEOUT_MAX_OPTION = "buildTimeoutMax";

public static final String INSTRUCTOR_BUILD_TIMEOUT_DEFAULT_OPTION = "buildTimeoutDefault";

public static final String USE_EXTERNAL = "useExternal";

public static final String EXTERNAL_CREDENTIAL_PROVIDER = "externalCredentialProvider";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LOCALCI;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.actuate.info.Info;
import org.springframework.boot.actuate.info.InfoContributor;
import org.springframework.context.annotation.Profile;
Expand All @@ -13,9 +14,23 @@
@Profile(PROFILE_LOCALCI)
public class LocalCIInfoContributor implements InfoContributor {

@Value("${artemis.continuous-integration.build-timeout-seconds.min:10}")
private int minInstructorBuildTimeoutOption;

@Value("${artemis.continuous-integration.build-timeout-seconds.max:240}")
private int maxInstructorBuildTimeoutOption;

@Value("${artemis.continuous-integration.build-timeout-seconds.default:120}")
private int defaultInstructorBuildTimeoutOption;

@Override
public void contribute(Info.Builder builder) {
// Store name of the continuous integration system
builder.withDetail(Constants.CONTINUOUS_INTEGRATION_NAME, "Local CI");

// Store the build timeout options for the instructor build
builder.withDetail(Constants.INSTRUCTOR_BUILD_TIMEOUT_MIN_OPTION, minInstructorBuildTimeoutOption);
builder.withDetail(Constants.INSTRUCTOR_BUILD_TIMEOUT_MAX_OPTION, maxInstructorBuildTimeoutOption);
builder.withDetail(Constants.INSTRUCTOR_BUILD_TIMEOUT_DEFAULT_OPTION, defaultInstructorBuildTimeoutOption);
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 4 additions & 1 deletion src/main/resources/config/application-buildagent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ artemis:
specify-concurrent-builds: false
concurrent-build-size: 1
asynchronous: true
timeout-seconds: 120
build-container-prefix: local-ci-
proxies:
use-system-proxy: false
Expand All @@ -34,6 +33,10 @@ artemis:
expiry-minutes: 5
cleanup-schedule-minutes: 60
pause-grace-period-seconds: 60
build-timeout-seconds:
min: 10
default: 120
max: 240
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
git:
name: Artemis
email: [email protected]
Expand Down
5 changes: 4 additions & 1 deletion src/main/resources/config/application-localci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ artemis:
# If true, the CI jobs will be executed asynchronously. If false, the CI jobs will be executed synchronously (e.g. for debugging and tests).
asynchronous: true
# The maximum number of seconds that a CI job is allowed to run. If the job exceeds this time, it will be terminated.
timeout-seconds: 120
# The number of builds that can be in the local CI queue at the same time. Choosing a small value can prevent the CI system from being overloaded on slow machines. Jobs that are submitted when the queue is already full, will be discarded.
queue-size-limit: 100
# The prefix that is used for the Docker containers that are created by the local CI system.
Expand All @@ -32,5 +31,9 @@ artemis:
expiry-days: 2
# Time of cleanup (cron expression)
cleanup-schedule-time: 0 0 3 * * *
build-timeout-seconds:
min: 10
default: 120
max: 240


Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
class="w-100 mt-1"
#timeoutField="ngModel"
type="range"
min="0"
max="120"
min="{{ timeoutMinValue }}"
max="{{ timeoutMaxValue }}"
step="1"
value="120"
value="{{ timeoutDefaultValue }}"
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
id="field_timeout"
[ngModel]="timeout()"
(ngModelChange)="timeoutChange!.emit($event)"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { Component, input, output, viewChild } from '@angular/core';
import { Component, OnInit, inject, input, output, viewChild } from '@angular/core';
import { NgModel } from '@angular/forms';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';

@Component({
selector: 'jhi-programming-exercise-build-configuration',
templateUrl: './programming-exercise-build-configuration.component.html',
styleUrls: ['../../../../programming-exercise-form.scss'],
})
export class ProgrammingExerciseBuildConfigurationComponent {
export class ProgrammingExerciseBuildConfigurationComponent implements OnInit {
private profileService = inject(ProfileService);

dockerImage = input.required<string>();
dockerImageChange = output<string>();

Expand All @@ -18,4 +21,29 @@ export class ProgrammingExerciseBuildConfigurationComponent {
dockerImageField = viewChild<NgModel>('dockerImageField');

timeoutField = viewChild<NgModel>('timeoutField');

timeoutMinValue?: number;
timeoutMaxValue?: number;
timeoutDefaultValue?: number;

ngOnInit() {
this.profileService.getProfileInfo().subscribe((profileInfo) => {
if (profileInfo) {
this.timeoutMinValue = profileInfo.buildTimeoutMin ?? 10;

// Set the maximum timeout value to 240 if it is not set in the profile or if it is less than the minimum value
this.timeoutMaxValue = profileInfo.buildTimeoutMax && profileInfo.buildTimeoutMax > this.timeoutMinValue ? profileInfo.buildTimeoutMax : 240;

// Set the default timeout value to 120 if it is not set in the profile or if it is not in the valid range
this.timeoutDefaultValue = 120;
if (profileInfo.buildTimeoutDefault && profileInfo.buildTimeoutDefault >= this.timeoutMinValue && profileInfo.buildTimeoutDefault <= this.timeoutMaxValue) {
this.timeoutDefaultValue = profileInfo.buildTimeoutDefault;
}

if (!this.timeout) {
this.timeoutChange.emit(this.timeoutDefaultValue);
}
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export class ProfileInfo {
public useVersionControlAccessToken?: boolean;
public showCloneUrlWithoutToken?: boolean;
public continuousIntegrationName?: string;
public buildTimeoutMin?: number;
public buildTimeoutMax?: number;
public buildTimeoutDefault?: number;
public programmingLanguageFeatures: ProgrammingLanguageFeature[];
public saml2?: Saml2Config;
public textAssessmentAnalyticsEnabled?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ export class ProfileService {

profileInfo.theiaPortalURL = data.theiaPortalURL ?? '';

profileInfo.buildTimeoutMin = data.buildTimeoutMin;
profileInfo.buildTimeoutMax = data.buildTimeoutMax;
profileInfo.buildTimeoutDefault = data.buildTimeoutDefault;
BBesrour marked this conversation as resolved.
Show resolved Hide resolved

return profileInfo;
}),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,22 @@ import { ArtemisTestModule } from '../../test.module';
import { ProgrammingExerciseBuildConfigurationComponent } from 'app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component';
import { FormsModule } from '@angular/forms';
import { ArtemisProgrammingExerciseUpdateModule } from 'app/exercises/programming/manage/update/programming-exercise-update.module';
import { of } from 'rxjs';
import { ProfileService } from '../../../../../main/webapp/app/shared/layouts/profiles/profile.service';

describe('ProgrammingExercise Docker Image', () => {
let comp: ProgrammingExerciseBuildConfigurationComponent;
let profileServiceMock: { getProfileInfo: jest.Mock };

beforeEach(() => {
profileServiceMock = {
getProfileInfo: jest.fn(),
};

TestBed.configureTestingModule({
imports: [ArtemisTestModule, FormsModule, ArtemisProgrammingExerciseUpdateModule],
declarations: [ProgrammingExerciseBuildConfigurationComponent],
providers: [],
providers: [{ provide: ProfileService, useValue: profileServiceMock }],
})
.compileComponents()
.then();
Expand All @@ -36,4 +43,44 @@ describe('ProgrammingExercise Docker Image', () => {
comp.timeoutChange.subscribe((value) => expect(value).toBe(20));
comp.timeoutChange.emit(20);
});

it('should set timeout options', () => {
profileServiceMock.getProfileInfo.mockReturnValue(
of({
buildTimeoutMin: undefined,
buildTimeoutMax: undefined,
buildTimeoutDefault: undefined,
}),
);

comp.ngOnInit();
expect(comp.timeoutMinValue).toBe(10);
expect(comp.timeoutMaxValue).toBe(240);
expect(comp.timeoutDefaultValue).toBe(120);

profileServiceMock.getProfileInfo.mockReturnValue(
of({
buildTimeoutMin: 0,
buildTimeoutMax: 360,
buildTimeoutDefault: 60,
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
}),
);
comp.ngOnInit();
expect(comp.timeoutMinValue).toBe(0);
expect(comp.timeoutMaxValue).toBe(360);
expect(comp.timeoutDefaultValue).toBe(60);

profileServiceMock.getProfileInfo.mockReturnValue(
of({
buildTimeoutMin: 100,
buildTimeoutMax: 20,
buildTimeoutDefault: 10,
}),
);

comp.ngOnInit();
expect(comp.timeoutMinValue).toBe(100);
expect(comp.timeoutMaxValue).toBe(240);
expect(comp.timeoutDefaultValue).toBe(120);
});
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
});
Loading