How ECS-related Design Should Be Like?

[t-kikuc]

Why

• Backgrounds:
  • Types of use cases are increasing, as more users use PipeCD for ECS.
    • We cannot support all of ELB use cases yet.
    • We’d like to support other than ELB - Service Discovery, Service Connect, and App Mesh.
  • ECS itself has evolved.
• Problems:
  • It's difficult to add features or refactor due to complexity.
  • Configs(.yaml) are getting complicated or non-intuitive for users.

What to discuss

• Share the points you'd like to improve, even if you don't have a clear solution.
  • Which codes should be refactored? Why?
    • e.g. We should create other stages, instead of using switch blocks.
  • Which configs/stages should be redesigned? Why?
    • e.g. input xxx should be placed in the yyy section with zzz option.

[t-kikuc]

First opinion: The stage ECS_TRAFFIC_ROUTING should be replaced by new stages.
For example, ELB_TRAFFIC_ROUTING, SERVICE_DISCOVERY_TRAFFIC_ROUTING, APP_MESH_TRAFFIC_ROUTING, and so on.

The reason: Maintainability

• Traffic routing depends on the type of networking and they use different logics and input configs.
  • e.g. We need to handle ELB APIs when using ELBs, but we don't need it when not using ELBs.
• We should separate them so that we can easily/separately maintain traffic routing logics, even if AWS's spec is changed.

[khanhtc1202]

Agree with this, instead of making common use ECS_TRAFFIC_ROUTING, case-specific stage would be a great improvement 👍

[khanhtc1202]

Another point to note here is that I think the current pre-prepare (aka. input section of ECS application containing so much unrelated things)

apiVersion: pipecd.dev/v1beta1
kind: ECSApp
spec:
  name: test
  input:
    serviceDefinitionFile: servicedef.yaml
    taskDefinitionFile: taskdef.yaml
    targetGroups:
      primary:
        targetGroupArn: arn:aws:elasticloadbalancing:ap-northeast-1:XXXX:targetgroup/ecs-tg-blue/YYYY
        containerName: web
        containerPort: 80
      canary:
        targetGroupArn: arn:aws:elasticloadbalancing:ap-northeast-1:XXXX:targetgroup/ecs-tg-green/ZZZZ
        containerName: web
        containerPort: 80

Targetgroup is the configuration for only ELB uses, so it would be better to move it to the ELB_TRAFFIC_ROUTING stage configuration. There is only one point here, which is we may need the targetgroup at the time we create taskset/task in stages before ELB_TRAFFIC_ROUTING, we should find a way to make this work smoothly 🤔

[khanhtc1202]

Should we draft some possible ELB_TRAFFIC_ROUTING configurations in case of using it? @t-kikuc

[t-kikuc]

Targetgroup is the configuration for only ELB uses, so it would be better to move it to the ELB_TRAFFIC_ROUTING stage configuration.

That's a good point.

Now I wonder if the new trafficRouting section for ECS like k8s would help us.
I need to compare such options.

apiVersion: pipecd.dev/v1beta1
kind: KubernetesApp
spec:
  name: mesh-istio-canary
  labels:
    env: example
    team: product
  pipeline:
    stages:
      - name: K8S_CANARY_ROLLOUT
        with:
          replicas: 10%
      - name: K8S_TRAFFIC_ROUTING
        with:
          primary: 90
          canary: 10
      - name: WAIT_APPROVAL
      - name: K8S_PRIMARY_ROLLOUT
      - name: K8S_TRAFFIC_ROUTING
        with:
          primary: 100
      - name: K8S_CANARY_CLEAN
  trafficRouting:   # HERE
    method: istio
    istio:
      host: mesh-istio-canary.default.svc.cluster.local

https://github.com/pipe-cd/pipecd/blob/301e3673f448b6a4d2e86921827b84c937d09002/examples/kubernetes/mesh-istio-canary/app.pipecd.yaml

[t-kikuc]

FYI: Current Supported Deployment Strategies.

type	QuickSync	Canary	Blue/Green
Standalone Task	supported	-	-
ELB	supported	supported	supported
Service Discovery	preview	preview	wip
Service Connect	-	-	-
App Mesh	-	-	-

[t-kikuc]

The issue #4710 is related.
We need to separate PrimaryRollout and CanaryClean.

[t-kikuc]

🗺️ The roadmap of redesigning/refactoring woule be like below, starting from not affecting users.
Of course, this is not the final version. If you have other points to redesign, please feel free to suggest.

1. Refactor current stages - does not affect app.pipecd.yaml
   • ELB's TRAFFIC_ROUTING (mainly [ECS] Support modifying listener rules other than defaults #4725)
   • PRIMARY_ROLLOUT ("We need to separate PrimaryRollout and CanaryClean.")
   • ROLLBACK (if necessary)
   • Now I don't think we need to add new stages.
2. Refactor app.pipecd.yaml - Add new configs, coexisting with current ones until migration.
   • Split input area.
     • Add trafficRouting area?
     • Add standaloneTask area? Because some of input configs are used only for StandaloneTask.
3. Support new types - If needed, we'll support earlier.
   • Service Discovery
   • Service Connect
   • App Mesh
   • They'll use the trafficRouting config area.

[t-kikuc]

By the way, before refactoring, I hope there would be more automated tests for AWS-related components.

[t-kikuc]

Add trafficRouting area?

This is because input.accessType is not smart... even though it's made by me

[t-kikuc]

2. Refactor app.pipecd.yaml

Current ECSDeploymentInput and the to-be are like this:

Field	Type	Should go to
serviceDefinitionFile	string	stay (input.serviceDefinitionFile)
taskDefinitionFile	string	stay (input.taskDefinitionFile)
autoRollback	bool	stay (input.autoRollback)
targetGroups	ECSTargetGroupInput	trafficRouting.elb.targetGroups
accessType	string	trafficRouting.method
runStandaloneTask	bool	standaloneTask.runTaskOnDeploy
clusterArn	string	standaloneTask.clusterArn
launchType	string	standaloneTask.launchType
awsVpcConfiguration	ECSVpcConfiguration	standaloneTask.awsvpcConfiguration

Only 3 fields will remain in the input area, which are necessary for almost all ECS application.

[t-kikuc]

Other problems users face: How to manage ECS resources - by ECS or Terraform?

Especially, it's difficult to manage depending resources. e.g. Applicaiton Autoscaling

[t-kikuc]

[Current Problem] We cannot specify tags in taskdef.yaml.

That's because types.TaskDefinition does not have Tags field.

[t-kikuc]

For standalone tasks, tags should be like this:

• Define tags in taskdef.yaml
• Propagate the tags of taskdef to tasks (by PropagateTags: TASK_DEFINITION in RunTaskInput??)

[t-kikuc]

Let me summarize so far.

Current Problems:

• Config
  • Some fields are not supported yet
    • and some of them are difficult to support (e.g. tags in taskdef)
  • Complicated app.pipecd.yaml
    • Some fields are only valid for standalone-tasks, which are noisy for services
  • Some traffic controller types are not supported yet (ServiceConnect, AppMesh)
• Behaviour
  • Some restrictions in DriftDetection (e.g. loadbalancers are not compared)
    • details: Support Drift Detection for ECS #5122
  • Unexpected 503 error (bug)
    • details: [ECS] 503 error occurred while PrimaryRollout because it deletes the canary too early #4710
  • LiveStates and DriftDetection are not supported for standalone-tasks yet.
• Code
  • If-states of standalone tasks are noisy for services

Note: Using AutoScaling was already supported #5030

[t-kikuc]

New Config Comparison

TaskDef: ecs.RegisterTaskDefinitionInput is better

Compare types.TaskDefinition and ecs.RegisterTaskDefinitionInput.

• Only types.TaskDefinition has:
  • Compatibilities
  • DeregisteredAt, RegistredAt, Revision, Status, TaskDefinitionArn: These are decided by ECS.
• Only ecs.RegisterTaskDefinitionInput has:
  • Tags

ServiceDef: ecs.CreateServiceInput is best

Compare types.Service, ecs.CreateServiceInput, and ecs.UpdateServiceInput.

Field	types.Service	ecs.CreateServiceInput	ecs.UpdateServiceInput
name of cluster	ClusterArn	Cluster	Cluster
name of service	ServiceName	ServiceName	Service
name of role	RoleArn	Role	-
type of DesiredCount	int32	*int32	*int32
type of EnableECSManagedTags	bool	*bool	bool
type of EnableExecuteCommand	bool	*bool	bool
DeploymentController	Yes	Yes	-
LaunchType	Yes	Yes	-
SchedulingStrategy	Yes	Yes	-
Tags	Yes	Yes	-
ServiceConnectConfiguration	-	Yes	Yes
VolumeConfigurations	-	Yes	Yes
Only it has	Deployments, Events, RunningCount, PendingCount, ServiceArn, Status, CreatedAt, CreatedBy, TaskSets	ClientToken	ForceNewDeployment

Fields that only Service has are not necessary for manifest because ECS decides them.