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

Adding Jenkins instance for Benchmark #381

Closed

Conversation

jordarlu
Copy link
Contributor

@jordarlu jordarlu commented Jan 5, 2024

Description

Adding Jenkins instance for Benchmark -

  • Necessary updates on existing opensearch-ci to share resources
  • Adding Benchmark instance

Issues Resolved

#382

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@jordarlu jordarlu marked this pull request as ready for review January 5, 2024 22:42
@jordarlu jordarlu marked this pull request as draft January 5, 2024 22:57
@jordarlu jordarlu marked this pull request as ready for review January 8, 2024 20:50
Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

I think the approach here needs to be revisited. As a user, I should be able to deploy any number of ci-config and ci stacks without having to create new classes everytime. See https://github.com/opensearch-project/opensearch-ci/blob/main/bin/ci-stack.ts for more details.
The defaultEnv can be replaced by any number of values by the user which can help to deploy same stacks for different environments, In this case, benchmarks.

With respect to the configuration customization, I can suggest below:

  1. Allow user to pass custom jenkins.yaml
  2. Allow user to specify what agent nodes needs to be deployed. Can be cateogorized into:
    a. Default Agent (Already Present)
    b. ProdAgents (Already Present, can be changed)
    c. Benchmarks Agents (Collection of all agent nodes related to benchmarks)
    d. All (All of the above)

I don't see any variable in the docker-compose file so maybe better to use default or allow user to specify its own.

Thanks!

@jordarlu
Copy link
Contributor Author

jordarlu commented Jan 8, 2024

I think the approach here needs to be revisited. As a user, I should be able to deploy any number of ci-config and ci stacks without having to create new classes everytime. See https://github.com/opensearch-project/opensearch-ci/blob/main/bin/ci-stack.ts for more details. The defaultEnv can be replaced by any number of values by the user which can help to deploy same stacks for different environments, In this case, benchmarks.

With respect to the configuration customization, I can suggest below:

  1. Allow user to pass custom jenkins.yaml
  2. Allow user to specify what agent nodes needs to be deployed. Can be cateogorized into:
    a. Default Agent (Already Present)
    b. ProdAgents (Already Present, can be changed)
    c. Benchmarks Agents (Collection of all agent nodes related to benchmarks)
    d. All (All of the above)

I don't see any variable in the docker-compose file so maybe better to use default or allow user to specify its own.

Thanks!

Thank for the comments, @gaiksaya , I may need to check further and discuss more on this becasue :

  1. User is instructed to do npm run cdk deploy on specific stack(s) at (https://github.com/opensearch-project/opensearch-ci/?tab=readme-ov-file#dev-deployment); In this case, I guess we will need to have the actual class files?
  2. On the internal CDK package where we planned to initiate new stacks to create new instances, I think that would require actual class files as well.
  3. For the users who wants to deploy CI in their enviroment, I think they might just need the existing/one CI instance instead of mutli instances ; I feel that the multi instances would be just for build team..

@gaiksaya
Copy link
Member

gaiksaya commented Jan 8, 2024

Thank for the comments, @gaiksaya , I may need to check further and discuss more on this becasue :

  1. User is instructed to do npm run cdk deploy on specific stack(s) at (https://github.com/opensearch-project/opensearch-ci/?tab=readme-ov-file#dev-deployment); In this case, I guess we will need to have the actual class files?

npm cdk deploy <stack_name> is the actual command. Hence if you have 10 stacks in same app, you can deploy all using npm cdk deploy "*" or just one stack using npm cdk deploy <stack_name>

  1. On the internal CDK package where we planned to initiate new stacks to create new instances, I think that would require actual class files as well.

We need to reuse the current stack to deploy a similar set up. An example would be initialize the stack for number of different use cases:

const app = new App();

const defaultEnv: string = 'Dev';
const defaultBenchmarkEnv: string = 'benchMark

const ciConfigStack = new CIConfigStack(app, `OpenSearch-CI-Config-${defaultEnv}`, {});
const ciConfigBenchStack = new CIConfigStack(app, `OpenSearch-CI-Config-${defaultBenchmarkEnv}`, {});

const ciStack = new CIStack(app, `OpenSearch-CI-${defaultEnv}`, {});
const ciBenchStack = new CIStack(app, `OpenSearch-CI-${defaultBenchmarkEnv}`, {});
  1. For the users who wants to deploy CI in their enviroment, I think they might just need the existing/one CI instance instead of mutli instances ; I feel that the multi instances would be just for build team..

Right, hence we would be keeping this code base as generic and minimal as possible. For user specific use case (like benchmarks) we will initialize as per many stacks as we want like above

@jordarlu
Copy link
Contributor Author

jordarlu commented Jan 9, 2024

  1. On the internal CDK package where we planned to initiate new stacks to create new instances, I think that would require actual class files as well.

We need to reuse the current stack to deploy a similar set up. An example would be initialize the stack for number of different use cases:

const app = new App();

const defaultEnv: string = 'Dev';
const defaultBenchmarkEnv: string = 'benchMark

const ciConfigStack = new CIConfigStack(app, `OpenSearch-CI-Config-${defaultEnv}`, {});
const ciConfigBenchStack = new CIConfigStack(app, `OpenSearch-CI-Config-${defaultBenchmarkEnv}`, {});

const ciStack = new CIStack(app, `OpenSearch-CI-${defaultEnv}`, {});
const ciBenchStack = new CIStack(app, `OpenSearch-CI-${defaultBenchmarkEnv}`, {});

How about having a 'template stack' for all new instance instead of making changes into the existing stack? for example :

const app = new App();

const defaultEnv: string = 'Dev';
const multiinstanceBenchmark: string = 'Benchmark';
const multiinstanceGradleCheck: string = 'GradleCheck';

const ciConfigStack = new CIConfigStack(app, `OpenSearch-CI-Config-${defaultEnv}`, {});
const ciConfigBenchStack = new CIConfigStackMultiInstance(app, `OpenSearch-CI-Config-${multiinstanceBenchmark}`, {});
const ciConfigGradleStack = new CIConfigStackMultiInstance(app, `OpenSearch-CI-Config-${multiinstanceGradleCheck}`, {});

const ciStack = new CIStack(app, `OpenSearch-CI-${defaultEnv}`, {});
const ciBenchStack = new CIStackMultiInstance(app, `OpenSearch-CI-${multiinstanceBenchmark}`, {});
const ciGradleStack = new CIStackMultiInstance(app, `OpenSearch-CI-${multiinstanceGradleCheck}`, {});

The reason is we can prevent more code update on the existing CIStack and CIConfigstack because we want to avoid rebuilding of exsiting CFN template ;
we will still have new class files to be used as the template for all new Jenkins instances ( multi-instances ) but just one set of the classes that will be applied to all new Jenkins instances

@jordarlu jordarlu marked this pull request as draft January 11, 2024 18:00
@jordarlu jordarlu closed this Feb 6, 2024
@jordarlu jordarlu deleted the jenkins_multi_instance branch March 22, 2024 03:47
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