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

Add support for multiple type of platforms #3817

Closed
fabidick22 opened this issue Mar 19, 2024 · 1 comment
Closed

Add support for multiple type of platforms #3817

fabidick22 opened this issue Mar 19, 2024 · 1 comment
Labels
abandoned enhancement New feature or request help wanted Extra attention is needed Stale

Comments

@fabidick22
Copy link

fabidick22 commented Mar 19, 2024

Overview

Currently the project works very well for creating runners with platforms like EC2, adding a new platform can be complicated due to the initial approach of how the project was developed.

Use Case

Add different types of platforms that allow us to reduce costs and execution times such as: AWS Fargate (Spot), AWS Lambda, etc.

Suggest a Fix

To ensure that the project support multiple types of platforms we have to refactor some parts of the code, this is a small analysis of what we have to update:

Possible conflicts with interfaces

This is a list of the data types that we need to update or standardize in order to support different types of platforms.

export type RunnerType = 'Org' | 'Repo';

We will need an interface similar to this one (RunnerType) to define the type of runners we are going to support. A name like RunnerInstanceType could be a good option, but this might create conflicts with the type of EC2 instances. Therefore, a more appropriate name could be RunnerPlatformType. This would look something like:

export type RunnerPlatformType = 'EC2' | 'Fargate';

export interface ListRunnerFilters {}

We need to add a new attribute to the interface because the way of listing/filtering EC2 instances is different for Fargate type instances.

export interface ListRunnerFilters {
  runnerType?: RunnerType;
+ runnerPlatformType?: RunnerPlatformType;
  runnerOwner?: string;
  environment?: string;
  statuses?: string[];
}

export interface RunnerInputParameters {...}

This interface is used in the CreateEC2RunnerConfig interface, and it seems to be duplicating some attributes like:

  • environment
  • subnets
  • launchTemplateName
  • ec2instanceCriteria
  • numberOfRunners
  • amiIdSsmParameterName
  • tracingEnabled
  • onDemandFailoverOnError

If we modify the RunnerInputParameters interface as a generic interface, we can use it to create interfaces for each type of runner (EC2, Fargate, etc.) that we need to configure. The following diagram shows the current state:

classDiagram
    RunnerInputParameters <|-- CreateEC2RunnerConfig
    class RunnerInputParameters{
        +String environment
        +RunnerType runnerType
        +String runnerOwner
        +String[] subnets
        +String launchTemplateName
        +Object ec2instanceCriteria
        +int numberOfRunners
        +String amiIdSsmParameterName
        +bool tracingEnabled
        +String onDemandFailoverOnError
    }
    class CreateEC2RunnerConfig{
        +String environment
        +String[] subnets
        +String launchTemplateName
        +RunnerInputParameters['ec2instanceCriteria'] ec2instanceCriteria
        +int numberOfRunners
        +String amiIdSsmParameterName
        +bool tracingEnabled
        +String[] onDemandFailoverOnError
    }
Loading

Proposal
The proposal would be to modify the RunnerInputParameters interface with parameters that are used by all runners and create a specific interface for each type of runner (EC2, Fargate, etc.).

classDiagram
    RunnerInputParameters <|-- CreateEC2RunnerConfig
    RunnerInputParameters <|-- CreateFargateRunnerConfig
    class RunnerInputParameters{
        +String environment
        +RunnerType runnerType
        +String runnerOwner
        +String[] subnets
        +int numberOfRunners
        +bool tracingEnabled
        +String onDemandFailoverOnError
    }
    class CreateEC2RunnerConfig{
        +RunnerInputParameters runnerInputsParams
        +String launchTemplateName: string;
        +String[] instanceTypes: string[];
        +DefaultTargetCapacityType targetCapacityType: DefaultTargetCapacityType;
        +String maxSpotPrice?: string;
        +SpotAllocationStrategy instanceAllocationStrategy: SpotAllocationStrategy;
        +String amiIdSsmParameterName?: string;
    }
    class CreateFargateRunnerConfig{
        +RunnerInputParameters runnerInputsParams
        +String taskDefinition
        +String clusterName
        +String[] securityGroups
    }
Loading

Code example:

export interface RunnerInputParameters {
 environment: string;
 runnerType: RunnerType;
 runnerOwner: string;
 subnets: string[];
- launchTemplateName: string;
- ec2instanceCriteria: {
-    instanceTypes: string[];
-    targetCapacityType: DefaultTargetCapacityType;
-    maxSpotPrice?: string;
-    instanceAllocationStrategy: SpotAllocationStrategy;
-  };
 numberOfRunners: number;
-  amiIdSsmParameterName?: string;
 tracingEnabled?: boolean;
 onDemandFailoverOnError?: string[];
}

EC2

export interface CreateEC2RunnerConfig {
 runnerInputsParams: RunnerInputParameters; // HERE
 
 launchTemplateName: string;
 instanceTypes: string[];
 targetCapacityType: DefaultTargetCapacityType;
 maxSpotPrice?: string;
 instanceAllocationStrategy: SpotAllocationStrategy;
 amiIdSsmParameterName?: string;
}

Fargate

export interface CreateFargateRunnerConfig {
 runnerInputsParams: RunnerInputParameters; // HERE
 
 taskDefinition: string;
 clusterName: string;
 securityGroups: string[];
 ...
}

Probable functions conflicts

This is a list of the functions that need to be updated to add support for Fargate instances.

export async function listEC2Runners() {} runners.ts

We need to rename the function listEC2Runners to something like listRunners(). This function receives an object of type ListRunnerFilters. This interface has already been modified to support instances of different types (EC2, Fargate).

export async function createRunner() {} runners.ts

We will need to update this function. To avoid a major refactoring, we can change the data type of runnerParameters so that it can receive an object of type EC2 or Fargate, something like:

export async function createRunner(runnerParameters: Runners.CreateEC2RunnerConfig | Runners.CreateFargateRunnerConfig): Promise<string[]> {
  ...
}

I'm not sure if this complicates things, but the other option is to create a function for each type of instance, something like:

export async function createEc2Runner() {}
export async function createFargateRunner() {}

async function createInstances() {} runners.ts

If we keep the function createRunner() with the same name, we'll probably need to rename this function to something like createEC2Instances(). This way, we can have a function to separate the logic of creating different types of instances (EC2, Fargate), something like:

export async function createRunner() {
	if(runnerPlatformType == "ec2"){
		createEC2Instances();
	} else {
		createFargateInstances();
	}
}
export async function createEC2Instances() {}
export async function createFargateInstances() {}

export async function terminateRunner() {} runners.ts

I don't see the need to rename the function, but we should add an attribute to identify how the runner will be deleted, in case it's an EC2 or Fargate runner, something like:

export async function terminateRunner(instanceId: string, runnerParameters: Runners.CreateEC2RunnerConfig | Runners.CreateFargateRunnerConfig): Promise<void> {
  ...
}
@npalm npalm added enhancement New feature or request help wanted Extra attention is needed labels Mar 21, 2024
Copy link
Contributor

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Apr 22, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned enhancement New feature or request help wanted Extra attention is needed Stale
Projects
None yet
Development

No branches or pull requests

2 participants