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

Restore s3 #47

Merged
merged 4 commits into from
Aug 17, 2016
Merged

Restore s3 #47

merged 4 commits into from
Aug 17, 2016

Conversation

noahdietz
Copy link
Contributor

@noahdietz noahdietz commented Aug 5, 2016

Fixes #48

  • restore the previously removed s3 read/write functionality for pod template spec storage
  • add validation for pod specs posted in this manner
  • add some dev Makefile targets and a dev deployment file

@mpnally
Copy link
Contributor

mpnally commented Aug 5, 2016

'restore' might have been a better word choice than 'replace'

@noahdietz noahdietz changed the title Replace s3 Restore s3 Aug 5, 2016
@noahdietz
Copy link
Contributor Author

replaced the test cases that were removed, passed test suite

@@ -12,6 +12,11 @@ import (
"k8s.io/kubernetes/pkg/api"
)

const (
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be environment variables because these values are configurable at the k8s-router level so we should make them configurable here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill add the fix

@whitlockjc
Copy link
Contributor

@tnine Do you mind reviewing this? The code looks fine but you're more familiar with the implementation than I.

* validates any pod template spec posted to S3 for `routable` `publicPaths` and an internal image URI
* add dev deployment file for quick in-cluster deving
* add dev Makefile targets
@tnine
Copy link
Contributor

tnine commented Aug 16, 2016

@whitlockjc @noahdietz This looks GTM. Aside from the env var comment @whitlockjc made, everything looks solid in this PR.

@noahdietz noahdietz merged commit 609a6c6 into master Aug 17, 2016
@noahdietz noahdietz deleted the replace-s3 branch August 17, 2016 17:41
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.

4 participants