-
Notifications
You must be signed in to change notification settings - Fork 221
Create Network FLow Monitor Addon #1212
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
base: main
Are you sure you want to change the base?
Conversation
3f51c74 to
5802945
Compare
|
@shapirov103 @elamaran11 functionally validated, had to make a bunch of changes to core addon and pod identity to make it work so please review! |
shapirov103
left a comment
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.
Looks great, a couple of minor comments/questions. Pending functional validation.
| } | ||
|
|
||
| /* VersioMap showing the default version for supported Kubernetes versions */ | ||
| const versionMap: Map<KubernetesVersion, string> = new Map([ |
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.
why no entry for 1.34? I see it in the matrix aws eks describe-addon-versions --addon-name aws-network-flow-monitoring-agent
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.
updated
| */ | ||
| export class EksPodIdentityAgentAddOn extends CoreAddOn { | ||
|
|
||
| @utils.conflictsWithAutoMode(utils.AutoModeConflictType.VERSION_MISMATCH, 'v1.3.4-eksbuild.1') |
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.
when running aws eks describe-addon-versions i see the following output:
- addonVersion: v1.0.2-eksbuild.6
architecture:
- amd64
- arm64
compatibilities:
- clusterVersion: '1.34'
defaultVersion: false
platformVersions:
- '*'
- ...
computeTypes:
- ec2
- auto
requiresConfiguration: false
requiresIamPermissions: truewe could incorporate these compute types and amd/arm support into the addon validation, e.g. auto mode compatibility for automode cluster provider and such.
It is not something to address in this PR, just a note to keep in mind.
shapirov103
left a comment
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.
minor comment
lib/utils/object-utils.ts
Outdated
| import { KubernetesVersion } from "aws-cdk-lib/aws-eks"; | ||
| import { cloneDeepWith } from 'lodash'; | ||
| import * as nutil from 'node:util/types'; | ||
| import { ManagedPolicyReference } from "aws-cdk-lib/aws-iam"; |
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.
it is from my commit iirc, let's remove this unused import, let's run make lint on the project
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.
done
shapirov103
left a comment
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.
LGTM
functional validation complete
❯ kubectl get pods -n amazon-network-flow-monitor
NAME READY STATUS RESTARTS AGE
aws-network-flow-monitor-agent-mfr86 1/1 Running 0 13m
╰─❯
@shapirov103 check logs if you can to make sure pod identity is working correctly. If not you'll see errors about OIDC stuff (before fixes to core-addon) |
@zjaco13 I don't see any errors in the log, all INFO messages. However, i do observe something odd: Why are there two associations, one is in kube-system? |
|
I had a failure on cluster delete, could be unrelated(subnet dependency), have you tried creating and dropping the stack? |
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.
@zjaco13 Nice work. Can you test with Graviton Nodes too?
Following needs to be added to ADOT Addon to scraper config to scrape the NFM Metrics.
- job_name: 'eks-nfm-agent'
kubernetes_sd_configs:
- role: pod
metrics_path: /metrics
relabel_configs:
- source_labels:
- __meta_kubernetes_namespace
- __meta_kubernetes_pod_label_name
action: keep
regex: amazon-network-flow-monitor;aws-network-flow-monitor-agent
- target_label: __address__
replacement: ${1}:9101
source_labels:
- __meta_kubernetes_pod_ip| /** | ||
| * Implementation of Network Flow Monitor Agent addon for EKS | ||
| */ | ||
| @utils.supportsALL |
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.
Did you test with Graviton Nodes?
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.
yes, tested on graviton!
@elamaran11 Added to AMP addon |
Believe unrelated, have had no issues with test stack |
Issue #, if available:
#1208
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.