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.

Sign up for GitHub

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

Jump to bottom

fix(eks): add clusterLogging props to Fargate Cluster #20707

Merged
merged 2 commits into from
Jun 13, 2022
Merged

fix(eks): add clusterLogging props to Fargate Cluster #20707

merged 2 commits into from
Jun 13, 2022

Conversation

ranjith-jagadeesh
Copy link
Contributor

@ranjith-jagadeesh ranjith-jagadeesh commented Jun 11, 2022 •
edited
Loading

Fixes #19302

All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Jun 11, 2022

@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Jun 11, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team June 11, 2022 13:34
@mergify
Copy link
Contributor

mergify bot commented Jun 13, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 1882d7c into aws:main Jun 13, 2022
@mergify
Copy link
Contributor

mergify bot commented Jun 13, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: f1e36b0
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

daschaa pushed a commit to daschaa/aws-cdk that referenced this pull request Jul 9, 2022
@ranjith-jagadeesh @daschaa
fix(eks): add clusterLogging props to Fargate Cluster (aws#20707)
2c113a7 
Fixes aws#19302
----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
guessi added a commit to guessi/aws-cdk that referenced this pull request Jul 17, 2022
@guessi
chore(eks): extend support for clusterLogging disable
45a5a18 
Amazon EKS Loggings was first introduced in aws#18112 and added Fargate
logging support in aws#20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: aws#19898
@guessi guessi mentioned this pull request Jul 17, 2022
Merged
4 tasks
guessi added a commit to guessi/aws-cdk that referenced this pull request Jul 17, 2022
@guessi
chore(eks): extend support for clusterLogging disable
c434913 
Amazon EKS Loggings was first introduced in aws#18112 and added Fargate
logging support in aws#20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: aws#19898
guessi added a commit to guessi/aws-cdk that referenced this pull request Jul 17, 2022
@guessi
chore(eks): extend support for clusterLogging disable
da58aa1 
Amazon EKS Loggings was first introduced in aws#18112 and added Fargate
logging support in aws#20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: aws#19898
guessi added a commit to guessi/aws-cdk that referenced this pull request Jul 17, 2022
@guessi
chore(eks): extend support for clusterLogging disable
cc7509d 
Amazon EKS Loggings was first introduced in aws#18112 and added Fargate
logging support in aws#20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: aws#19898
guessi added a commit to guessi/aws-cdk that referenced this pull request Jul 24, 2022
@guessi
chore(eks): extend support for clusterLogging disable
95ef236 
Amazon EKS Loggings was first introduced in aws#18112 and added Fargate
logging support in aws#20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: aws#19898
mergify bot pushed a commit that referenced this pull request Aug 1, 2022
@guessi
fix(eks): cannot disable cluster logging once it has been enabled (#2…
e41b073 
…1185)

### Description

Amazon EKS Loggings was first introduced in #18112 and added Fargate logging support in #20707. However, disabled loggings was not take into consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement. Also, added test cases for no logging configured, partial logging configured, full logging configured.

Fixes: #19898

### Test Results

#### Tool versions

```sh
$ node --version
v16.16.0
```

```sh
$ yarn --version
1.22.19
```

#### yarn build

```sh
$ yarn build
yarn run v1.22.19
$ cdk-build
...

Build times for @aws-cdk/aws-eks: Total time (59.5s) | /home/ec2-user/aws-cdk/tools/@aws-cdk/cdk-build-tools/node_modules/jsii/bin/jsii (35.0s) | cfn2ts (0.6s)
Done in 59.94s.
```

#### yarn test

```sh
$ yarn test

yarn run v1.22.19
$ cdk-test
PASS test/cluster.test.js (33.488 s)
PASS test/nodegroup.test.js (10.465 s)
PASS test/helm-chart.test.js (5.964 s)
PASS test/fargate.test.js (5.391 s)
PASS test/k8s-manifest.test.js
PASS test/service-account.test.js
PASS test/user-data.test.js
PASS test/awsauth.test.js
PASS test/alb-controller.test.js
PASS test/k8s-patch.test.js
...

=============================== Coverage summary ===============================
Statements   : 93.08% ( 888/954 )
Branches     : 89.47% ( 442/494 )
Functions    : 94.03% ( 142/151 )
Lines        : 94.83% ( 881/929 )
================================================================================

Test Suites: 13 passed, 13 total
Tests:       277 passed, 277 total
Snapshots:   0 total
Time:        77.227 s, estimated 91 s
Ran all test suites.

Verifying integration test snapshots...

  UNCHANGED  integ.eks-oidc-provider 27.294s
  UNCHANGED  integ.fargate-cluster 35.153s
  UNCHANGED  integ.eks-cluster-private-endpoint 35.223s
  UNCHANGED  integ.eks-helm-asset 35.602s
  UNCHANGED  integ.eks-bottlerocket-ng 35.981s
  UNCHANGED  integ.eks-cluster-handlers-vpc 36.266s
  UNCHANGED  integ.eks-inference 36.801s
  UNCHANGED  integ.alb-controller 37.565s
  UNCHANGED  integ.eks-cluster 38.041s

Snapshot Results:

Tests:    9 passed, 9 total
Tests successful. Total time (1m59.1s) | /home/ec2-user/aws-cdk/node_modules/jest/bin/jest.js (1m18.4s) | integ-runner (40.6s)
Done in 119.26s.
```

#### yarn integ

```sh
$ yarn integ
yarn run v1.22.19
$ integ-runner

Verifying integration test snapshots...

  UNCHANGED  integ.eks-oidc-provider 25.049s
  UNCHANGED  integ.eks-bottlerocket-ng 34.895s
  UNCHANGED  integ.eks-cluster-private-endpoint 35.012s
  UNCHANGED  integ.fargate-cluster 35.417s
  UNCHANGED  integ.eks-cluster-handlers-vpc 35.563s
  UNCHANGED  integ.eks-helm-asset 35.691s
  UNCHANGED  integ.eks-inference 36.303s
  UNCHANGED  integ.alb-controller 37.071s
  UNCHANGED  integ.eks-cluster 37.843s

Snapshot Results:

Tests:    9 passed, 9 total
Done in 40.59s.
```

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [X] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request Aug 30, 2022
@guessi @josephedward
fix(eks): cannot disable cluster logging once it has been enabled (aw…
a73dbda 
…s#21185)

### Description

Amazon EKS Loggings was first introduced in aws#18112 and added Fargate logging support in aws#20707. However, disabled loggings was not take into consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement. Also, added test cases for no logging configured, partial logging configured, full logging configured.

Fixes: aws#19898

### Test Results

#### Tool versions

```sh
$ node --version
v16.16.0
```

```sh
$ yarn --version
1.22.19
```

#### yarn build

```sh
$ yarn build
yarn run v1.22.19
$ cdk-build
...

Build times for @aws-cdk/aws-eks: Total time (59.5s) | /home/ec2-user/aws-cdk/tools/@aws-cdk/cdk-build-tools/node_modules/jsii/bin/jsii (35.0s) | cfn2ts (0.6s)
Done in 59.94s.
```

#### yarn test

```sh
$ yarn test

yarn run v1.22.19
$ cdk-test
PASS test/cluster.test.js (33.488 s)
PASS test/nodegroup.test.js (10.465 s)
PASS test/helm-chart.test.js (5.964 s)
PASS test/fargate.test.js (5.391 s)
PASS test/k8s-manifest.test.js
PASS test/service-account.test.js
PASS test/user-data.test.js
PASS test/awsauth.test.js
PASS test/alb-controller.test.js
PASS test/k8s-patch.test.js
...

=============================== Coverage summary ===============================
Statements   : 93.08% ( 888/954 )
Branches     : 89.47% ( 442/494 )
Functions    : 94.03% ( 142/151 )
Lines        : 94.83% ( 881/929 )
================================================================================

Test Suites: 13 passed, 13 total
Tests:       277 passed, 277 total
Snapshots:   0 total
Time:        77.227 s, estimated 91 s
Ran all test suites.

Verifying integration test snapshots...

  UNCHANGED  integ.eks-oidc-provider 27.294s
  UNCHANGED  integ.fargate-cluster 35.153s
  UNCHANGED  integ.eks-cluster-private-endpoint 35.223s
  UNCHANGED  integ.eks-helm-asset 35.602s
  UNCHANGED  integ.eks-bottlerocket-ng 35.981s
  UNCHANGED  integ.eks-cluster-handlers-vpc 36.266s
  UNCHANGED  integ.eks-inference 36.801s
  UNCHANGED  integ.alb-controller 37.565s
  UNCHANGED  integ.eks-cluster 38.041s

Snapshot Results:

Tests:    9 passed, 9 total
Tests successful. Total time (1m59.1s) | /home/ec2-user/aws-cdk/node_modules/jest/bin/jest.js (1m18.4s) | integ-runner (40.6s)
Done in 119.26s.
```

#### yarn integ

```sh
$ yarn integ
yarn run v1.22.19
$ integ-runner

Verifying integration test snapshots...

  UNCHANGED  integ.eks-oidc-provider 25.049s
  UNCHANGED  integ.eks-bottlerocket-ng 34.895s
  UNCHANGED  integ.eks-cluster-private-endpoint 35.012s
  UNCHANGED  integ.fargate-cluster 35.417s
  UNCHANGED  integ.eks-cluster-handlers-vpc 35.563s
  UNCHANGED  integ.eks-helm-asset 35.691s
  UNCHANGED  integ.eks-inference 36.303s
  UNCHANGED  integ.alb-controller 37.071s
  UNCHANGED  integ.eks-cluster 37.843s

Snapshot Results:

Tests:    9 passed, 9 total
Done in 40.59s.
```

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [X] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corymhall corymhall corymhall approved these changes

Assignees
No one assigned
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(aws-eks): Can't enable control plane logging for fargate cluster
3 participants
@ranjith-jagadeesh @aws-cdk-automation @corymhall