Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Bypass userdata limit with s3 plus follow ups #268

Conversation

mumoshu
Copy link
Contributor

@mumoshu mumoshu commented Jan 19, 2017

@redbaron FYI this is what I have so far.

Unit tests are failing/won't even compile but I've tested this by creating an actual cluster including 1 main cluster with 2 node pools including 1 asg-backed pool and 1 spot-fleet-backed pool.

If interested, please see my latest commit mumoshu@c2358d7 for what were changed since your work. Basically I've refactored cfnstack/ so that

  • you don't need explicitly render stack templates where it seems unnecessary (e.g. destroy.go) anymore
  • you don't need to pass s3URI all over the places

@mumoshu
Copy link
Contributor Author

mumoshu commented Jan 19, 2017

This isn't really a natural order of things I would do to fix something but anyways E2E tests are passing against the cluster.
I'll work on fixing unit tests next.

"stack.json": stackTemplate,
"userdata-controller": c.UserDataController,
"userdata-worker": c.UserDataWorker,
"userdata-etcd": c.UserDataEtcd,
Copy link
Contributor

Choose a reason for hiding this comment

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

etcd userdata can't be moved to s3, have to stay in CF template to be excluded from updates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I'll fix it. Anyways, it is uploaded but not read from anywhere now.

@mumoshu
Copy link
Contributor Author

mumoshu commented Jan 19, 2017

nodepools should write to a different path , otherwise the'll overwrite main cluster userdata. this probably means that --s3-uri arg should be moved into cluster.yaml config option

Thanks for the notice 👍
I'm aware of it and actually it is already addressed in this PR.

@codecov-io
Copy link

codecov-io commented Jan 20, 2017

Current coverage is 57.20% (diff: 38.68%)

No coverage report found for master at c538e61.

Powered by Codecov. Last update c538e61...7102d59

@mumoshu
Copy link
Contributor Author

mumoshu commented Jan 20, 2017

Unit/integration tests are passing.


templateURL, err := c.UploadTemplate(s3Svc, s3URI, stackBody)
if err != nil {
func (c *Provisioner) uploadTemplates(s3Svc S3ObjectPutterService, uploads map[string]string) (*string, error) {
Copy link
Contributor Author

@mumoshu mumoshu Jan 20, 2017

Choose a reason for hiding this comment

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

@redbaron It seems to me that you've used uploadTemplates to upload not only a template but also files referenced from it(e.g. cloud-config-worker and cloud-config-controller).
If so, I'd rather like to change its signature to something like uploadStackAssets(s3Svc S3ObjectPutterService, stackTemplate string, cloudConfigs: map[string]string) for now and extend/introduce a type like StackAssets in the future.
Then, if filename == "stack.json" { can be removed hence less complexity.

@mumoshu
Copy link
Contributor Author

mumoshu commented Jan 20, 2017

E2E tests passed.

Although I can merge this anytime, I'd like to hold on until other big PRs like #169 #227, which are likely to conflict with this PR, become ready to be merged.

@redbaron
Copy link
Contributor

redbaron commented Jan 20, 2017

I tested 9fce0cb change locally,seems to work, consider adding here if you agree

EDIT: just noticed it misses comment with userdata hash to trigger userdata update when real userdata changes on s3 bucket :)

@mumoshu
Copy link
Contributor Author

mumoshu commented Jan 24, 2017

@redbaron Thanks for wrapping it into a systemd unit 👍 I will test it out later.

Btw, kube-aws up seems to take 2x more time than it had taken before while creating ASGs:

image

Any idea how we could improve it?

@mumoshu mumoshu changed the title WIP: Bypass userdata limit with s3 plus follow ups Bypass userdata limit with s3 plus follow ups Jan 24, 2017
@mumoshu mumoshu added this to the v0.9.4-rc.1 milestone Jan 24, 2017
@mumoshu mumoshu merged commit f0bfebb into kubernetes-retired:master Jan 24, 2017
@mumoshu mumoshu deleted the bypass-userdata-limit-with-s3-plus-follow-ups branch January 24, 2017 13:14
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
…a-limit-with-s3-plus-follow-ups

Bypass userdata limit with s3 plus follow ups
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants