-
Notifications
You must be signed in to change notification settings - Fork 204
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
Flagger addon #440
base: main
Are you sure you want to change the base?
Flagger addon #440
Changes from 7 commits
25a5798
85c3fd9
159cb5a
050652a
81916a4
9e1705c
9d2db1f
79135f2
2cca7a8
39215e8
8459432
4ac7046
5768589
4361c80
4efc9bb
1a1a313
12da84c
795899c
6a788b2
3b7a146
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
apiVersion: autoscaling/v2beta2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this file for ? why is it in the source control? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was in the file example I got this from and figured an example api version in the file wouldn't hurt the test I was using it for. And I have not deleted those temporary yaml files yet, I can do that now. |
||
kind: HorizontalPodAutoscaler | ||
metadata: | ||
name: podinfo | ||
spec: | ||
scaleTargetRef: | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
name: podinfo | ||
minReplicas: 2 | ||
maxReplicas: 4 | ||
metrics: | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
target: | ||
type: Utilization | ||
# scale up if usage is above | ||
# 99% of the requested CPU (100m) | ||
averageUtilization: 99 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import 'source-map-support/register'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need this import. Why was it added? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure might have been from a quick fix I did but it was not faded so I assumed it was doing something. I have deleted it and see it did not mess anything up. |
||
import * as blueprints from '../../../lib'; | ||
import { Construct } from 'constructs'; | ||
|
||
/** | ||
shapirov103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* User provided options for the Helm Chart | ||
*/ | ||
export interface FlaggerAddOnProps extends blueprints.HelmAddOnUserProps {//this is the root level | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove the comment // this is the root level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was a personal note I forgot to remove, thanks for the catch. |
||
prometheusInstall?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's rename to https://github.com/aws-quickstart/cdk-eks-blueprints/blob/main/lib/addons/appmesh/index.ts#L17 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense. |
||
//meshProvider?: //need an enums for what you put from values; | ||
|
||
} | ||
|
||
/** | ||
shapirov103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Default props to be used when creating the Helm chart | ||
*/ | ||
export const defaultProps: blueprints.HelmAddOnProps & FlaggerAddOnProps = { | ||
name: "flagger", | ||
namespace: "flagger", | ||
chart: "flagger", | ||
version: "1.22.0", | ||
release: "flagger", | ||
repository: "https://flagger.app", | ||
values: {}, | ||
}; | ||
|
||
/** | ||
* Main class to instantiate the Helm chart | ||
*/ | ||
export class FlaggerAddOn extends blueprints.HelmAddOn { | ||
|
||
readonly options: FlaggerAddOnProps; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. line break here. |
||
|
||
constructor(props?: FlaggerAddOnProps) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. line break here. |
||
super({ ...defaultProps, ...props }); //merges your stuff and what they specify. They override our stuff, root level, and values properties | ||
this.options = this.props as FlaggerAddOnProps; | ||
} | ||
|
||
deploy(clusterInfo: blueprints.ClusterInfo): Promise<Construct> { | ||
const chart = this.addHelmChart(clusterInfo, defaultProps.values); | ||
|
||
return Promise.resolve(chart); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
apiVersion: v1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please drop all redundant files. |
||
kind: Service | ||
metadata: | ||
name: flagger-loadtester | ||
labels: | ||
app: flagger-loadtester | ||
spec: | ||
type: ClusterIP | ||
selector: | ||
app: flagger-loadtester | ||
ports: | ||
- name: http | ||
port: 80 | ||
protocol: TCP | ||
targetPort: http |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
apiVersion: flagger.app/v1beta1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this file |
||
kind: Canary | ||
metadata: | ||
name: podinfo | ||
namespace: test | ||
spec: | ||
# service mesh provider can be: kubernetes, istio, appmesh, nginx, gloo | ||
provider: kubernetes | ||
# deployment reference | ||
targetRef: | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
name: podinfo | ||
# the maximum time in seconds for the canary deployment | ||
# to make progress before rollback (default 600s) | ||
progressDeadlineSeconds: 60 | ||
# HPA reference (optional) | ||
autoscalerRef: | ||
apiVersion: autoscaling/v2beta2 | ||
kind: HorizontalPodAutoscaler | ||
name: podinfo | ||
service: | ||
port: 9898 | ||
portDiscovery: true | ||
analysis: | ||
# schedule interval (default 60s) | ||
interval: 30s | ||
# max number of failed checks before rollback | ||
threshold: 2 | ||
# number of checks to run before rollback | ||
iterations: 10 | ||
# Prometheus checks based on | ||
# http_request_duration_seconds histogram | ||
metrics: | ||
- name: request-success-rate | ||
# minimum req success rate (non 5xx responses) | ||
# percentage (0-100) | ||
thresholdRange: | ||
min: 99 | ||
interval: 1m | ||
- name: request-duration | ||
# maximum req duration P99 | ||
# milliseconds | ||
thresholdRange: | ||
max: 500 | ||
interval: 30s | ||
# acceptance/load testing hooks | ||
webhooks: | ||
- name: smoke-test | ||
type: pre-rollout | ||
url: http://flagger-loadtester.test/ | ||
timeout: 15s | ||
metadata: | ||
type: bash | ||
cmd: "curl -sd 'anon' http://podinfo-canary.test:9898/token | grep token" | ||
- name: load-test | ||
url: http://flagger-loadtester.test/ | ||
timeout: 5s | ||
metadata: | ||
type: cmd | ||
cmd: "hey -z 1m -q 10 -c 2 http://podinfo-canary.test:9898/" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { ArnPrincipal } from 'aws-cdk-lib/aws-iam'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this file here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops when I was trying to figure stuff out for patterns and add a team for it I think I put this into the wrong program when I swapped between them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is still in the code, why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is gone now, guess I got distracted by other issues and never got around to deleting it. I have done it now though. |
||
import { ApplicationTeam } from '../../../lib/teams'; | ||
|
||
export class TeamApplication extends ApplicationTeam { | ||
constructor(name: string, accountID: string) { | ||
super({ | ||
name: name, | ||
users: [new ArnPrincipal(`arn:aws:iam::${accountID}:user/application`)] | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
{ | ||
"name": "@aws-quickstart/eks-blueprints", | ||
"version": "1.0.4", | ||
"version": "1.0.5", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.1.1 |
||
"license": "Apache-2.0", | ||
"main": "dist/index.js", | ||
"types": "dist/index.d.ts", | ||
|
@@ -28,6 +28,7 @@ | |
"typescript": "~4.5.4" | ||
}, | ||
"dependencies": { | ||
"@aws-quickstart/eks-blueprints": "file:/Users/pevetoej/environment/cdk-eks-blueprints/aws-quickstart-eks-blueprints-1.0.5.tgz", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. drop this change, it was needed for the patterns repo and we made a mistake to run npm i here. |
||
"@types/assert": "^1.5.6", | ||
"@types/bcrypt": "^5.0.0", | ||
"@types/lodash.clonedeep": "^4.5.7", | ||
|
@@ -43,4 +44,4 @@ | |
"yaml": "^1.10.2", | ||
"zod": "^3.17.3" | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the GitHub actions warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean by that? Are you saying I should have it instead of ../../lib be the
import * as blueprints from '@aws-quickstart/eks-blueprints'
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use file view : https://github.com/aws-quickstart/cdk-eks-blueprints/pull/440/files#diff-40d4a72c979379c5b6ab47139c410af18bd607fd5b4f8a26d1aaef9956d8d447
scroll down from my comment and observe github actions warnings.