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

refresh subnet/CIDR information periodically #903

Merged
merged 2 commits into from Jun 24, 2020
Merged

Conversation

nithu0115
Copy link
Contributor

Issue #, if available: #737

Description of changes:

  1. Added forever go-routine to update VPC CIDR blocks by checking EC2 instance metadata every 30 seconds
  2. Added forever go-routine to update ip rules only if there is a new VPC CIDR added for existing pods to enable connectivity between pods running on VPC CIDR1(old) on eth1 to pods running on VPC CIDR2 (new) on eth1.

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

pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
@nithu0115 nithu0115 force-pushed the master branch 3 times, most recently from 7e82e55 to 3c40a6d Compare April 17, 2020 04:43
Copy link
Contributor Author

@nithu0115 nithu0115 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogren, addressed all your comments.

@nithu0115 nithu0115 requested a review from mogren April 17, 2020 13:02
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be somewhat tricky to get right... Some more changes needed.

pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
@nithu0115 nithu0115 force-pushed the master branch 3 times, most recently from 7e3075f to 7996e40 Compare May 5, 2020 18:49
Copy link
Contributor Author

@nithu0115 nithu0115 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, addressed all your comments @mogren except one.

pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
@nithu0115 nithu0115 requested a review from mogren May 5, 2020 19:03
Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting closer! Some changes still needed though.

pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/ipamd/ipamd.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@nithu0115 nithu0115 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogren , Addressed your comments :)

pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
@nithu0115 nithu0115 force-pushed the master branch 4 times, most recently from 989993a to 63518d1 Compare June 15, 2020 03:50
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nithu0115 still needs a bit more work, please! See inline...

pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
@nithu0115 nithu0115 force-pushed the master branch 6 times, most recently from 3e77e97 to 927c95b Compare June 18, 2020 19:16
Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the integration tests don't pass...

[It] should enable pod-pod communication
  /go/src/github.com/{{ORG_NAME}}/{{REPO_NAME}}/test/integration/integration_test.go:74
Jun 18 20:18:44.667: INFO: Unexpected error occurred: timed out waiting for the condition
[AfterEach] [cni-integration]
  /go/pkg/mod/k8s.io/kubernetes@v1.15.3/test/e2e/framework/framework.go:151
Jun 18 20:18:44.667: INFO: Waiting up to 3m0s for all (but 0) nodes to be ready
Jun 18 20:18:44.825: INFO: Found DeleteNamespace=false, skipping namespace deletion!

• Failure [302.416 seconds]
[cni-integration]
/go/src/github.com/{{ORG_NAME}}/{{REPO_NAME}}/test/integration/integration_test.go:70
  should enable pod-pod communication [It]
  /go/src/github.com/{{ORG_NAME}}/{{REPO_NAME}}/test/integration/integration_test.go:74

  waiting for pod running
  Unexpected error:
      <*errors.errorString | 0xc0002a5a20>: {
          s: "timed out waiting for the condition",
      }
      timed out waiting for the condition
  occurred

Does it work in your cluster without any issues?

pkg/awsutils/awsutils.go Show resolved Hide resolved
@nithu0115 nithu0115 force-pushed the master branch 3 times, most recently from 431542b to 01c2d5d Compare June 18, 2020 23:56
@nithu0115 nithu0115 requested a review from mogren June 18, 2020 23:57
Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nithu0115 nithu0115 requested a review from jaypipes June 19, 2020 02:52
@mogren mogren added this to the v1.7.0 milestone Jun 24, 2020
Copy link
Contributor

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. A few minor suggestions that you can add in a followup PR :) Thanks for your patience and hanging in there with this, @nithu0115!

pkg/awsutils/awsutils.go Show resolved Hide resolved
pkg/awsutils/awsutils.go Show resolved Hide resolved
pkg/awsutils/awsutils.go Show resolved Hide resolved
pkg/ipamd/ipamd.go Show resolved Hide resolved
@jaypipes jaypipes merged commit a073d66 into aws:master Jun 24, 2020
@mogren mogren mentioned this pull request Jun 28, 2020
bnapolitan added a commit to bnapolitan/amazon-vpc-cni-k8s that referenced this pull request Jul 1, 2020
commit d938e5e
Author: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
Date:   Wed Jul 1 01:19:14 2020 +0000

    Json o/p for logs from entrypoint.sh

commit 2d20308
Author: Nathan Prabhu <natprabh@amazon.com>
Date:   Mon Jun 29 18:06:22 2020 -0500

    bugfix: make metrics-helper docker logging statement multi-arch compatible

commit bf9ded3
Author: Claes Mogren <claes.mogren@gmail.com>
Date:   Sat Jun 27 14:51:35 2020 -0700

    Use install command instead of cp

commit e3b7dbb
Author: Gyuho Lee <leegyuho@amazon.com>
Date:   Mon Jun 29 09:40:02 2020 -0700

    scripts/lib: bump up tester to v1.4.0

    Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

commit c369480
Author: Claes Mogren <claes.mogren@gmail.com>
Date:   Sun Jun 28 12:19:27 2020 -0700

    Some refresh cleanups

commit 8c266e9
Author: Claes Mogren <claes.mogren@gmail.com>
Date:   Sun Jun 28 18:37:46 2020 -0700

    Run staticcheck and clean up

commit 8dfc5b1
Author: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
Date:   Sun Jun 28 17:39:20 2020 -0700

    Fix integration test script for code pipeline (aws#1062)

    Co-authored-by: Claes Mogren <mogren@amazon.com>

commit 52306be
Author: Murcherla <nithu0115@gmail.com>
Date:   Wed Jun 24 23:37:24 2020 -0500

    minor nits, fast follow up to PR 903

commit 4ddd248
Author: Claes Mogren <mogren@amazon.com>
Date:   Sun Jun 14 23:20:22 2020 -0700

    Add bandwidth plugin

commit 6d35fda
Author: Robert Sheehy <gameboy1092@gmail.com>
Date:   Fri May 22 21:11:12 2020 -0500

    Chain interface to other CNI plugins

commit 30f98bd
Author: Penugonda <saiteja313@gmail.com>
Date:   Thu Jun 25 15:14:00 2020 -0400

    removed custom networking default vars, introspection var

commit aa8b818
Author: Penugonda <saiteja313@gmail.com>
Date:   Wed Jun 24 19:11:38 2020 -0400

    updated manifest configs with default env vars

commit a073d66
Author: Nithish Murcherla <nithu0115@gmail.com>
Date:   Wed Jun 24 16:51:38 2020 -0500

    refresh subnet/CIDR information every 30 seconds and update ip rules to map pods (aws#903)

    Co-authored-by: Claes Mogren <mogren@amazon.com>

commit a0da387
Author: Claes Mogren <mogren@amazon.com>
Date:   Wed Jun 24 12:30:45 2020 -0700

    Default to random-fully (aws#1048)

commit 9fea153
Author: Claes Mogren <mogren@amazon.com>
Date:   Sun Jun 14 22:37:10 2020 -0700

    Update probe settings

    * Reduce readiness probe startup delay
    * Increase liveness polling period
    * Reduce shutdown grace period to 10 seconds

commit ad7df34
Author: Jay Pipes <jaypipes@gmail.com>
Date:   Wed Jun 24 02:06:23 2020 -0400

    Remove timeout for ipamd startup (aws#874)

    * add configurable timeout for ipamd startup

    Adds a configurable timeout to the aws-k8s-agent (ipamd) startup in the
    entrypoint.sh script. Increases the default timeout from ~30 seconds to
    60 seconds.

    Users can set the IPAMD_TIMEOUT_SECONDS environment variable to change
    the timeout.

    Related: aws#625, aws#865 aws#872

    * This is a local gRPC call, so just try every 1 second indefinitely

    Since we have a liveness probe restarting the probe, we can rely on that to kill the pod.

    Co-authored-by: Claes Mogren <mogren@amazon.com>

commit 1af40d2
Author: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
Date:   Fri Jun 19 10:14:44 2020 -0700

    Changelog and config file changes for v1.6.3

commit 14d5135
Author: Ari Becker <ari-becker@users.noreply.github.com>
Date:   Wed Jun 17 09:39:21 2020 +0300

    Generated the different configurations

commit 00395cb
Author: Ari Becker <ari-becker@users.noreply.github.com>
Date:   Tue Jun 16 14:33:55 2020 +0300

    Fix discovery RBAC issues in Kubernetes 1.17

commit 7e224af
Author: Gyuho Lee <leegyuho@amazon.com>
Date:   Mon Jun 15 16:04:44 2020 -0700

    scripts/lib/aws: bump up tester to v1.3.9

    Includes improvements to log fetcher + MNG deletion when metrics server
    is installed.

    Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

commit 36286ba
Author: Claes Mogren <mogren@amazon.com>
Date:   Mon Jun 15 07:56:59 2020 -0700

    Remove Printf and format test (aws#1027)

commit af54066
Author: Gyuho Lee <leegyuho@amazon.com>
Date:   Sat Jun 13 01:31:08 2020 -0700

    scripts/lib/aws: tester v1.3.6, enable color outputs (aws#1025)

    Includes various bug fixes + color output if $TERM is supported.
    Fallback to plain text output automatic.

    ref.
    https://github.com/aws/aws-k8s-tester/blob/master/CHANGELOG/CHANGELOG-1.3.md#v136-2020-06-12

    Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

commit 6d52e1b
Author: jayanthvn <1111446+jayanthvn@users.noreply.github.com>
Date:   Fri Jun 12 16:26:33 2020 -0700

    added warning if delete on termination is set to false for the primar… (aws#1024)

    * Added a warning message if delete on termination is set to false for the primary ENI
@@ -347,20 +373,92 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata() error {
}
log.Debugf("Found vpc-ipv4-cidr-block: %s ", cache.vpcIPv4CIDR)

// retrieve vpc-ipv4-cidr-blocks
// initSGIDs retrieves security groups
err = cache.refreshSGIDs(mac)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not invoked for CustomNetworking right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it is not!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants