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

Add init containers to pods #23567

Merged
merged 6 commits into from
May 18, 2016

Conversation

smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Mar 29, 2016

This implements #1589 as per proposal #23666

Incorporates feedback on #1589, creates parallel structure for InitContainers and Containers, adds validation for InitContainers that requires name uniqueness, and comments on a number of implications of init containers.

This is a complete alpha implementation.


This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 29, 2016
@smarterclayton smarterclayton mentioned this pull request Mar 29, 2016
@smarterclayton smarterclayton changed the title Adding init containers to pods WIP - Adding init containers to pods Mar 29, 2016
@bgrant0607
Copy link
Member

I would have preferred #831 first.

@@ -1429,6 +1429,20 @@ type PodSpec struct {
// List of volumes that can be mounted by containers belonging to the pod.
// More info: http://releases.k8s.io/HEAD/docs/user-guide/volumes.md
Volumes []Volume `json:"volumes,omitempty" patchStrategy:"merge" patchMergeKey:"name"`
// List of initialization containers belonging to the pod.
// Init containers are executed in order prior to containers being started. If any
// init container fails, the pod is considered to have failed and is handled according
Copy link
Member

Choose a reason for hiding this comment

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

Failed pods aren't restarted by Kubelet. Only failed containers are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bgrant0607
Copy link
Member

Looks ok. Not the approach I'd recommend for initializing volumes.

@smarterclayton
Copy link
Contributor Author

Specifically because? I'm envisioning the "filling volumes" usecase
much more than custom volume behaviors (fuse, etc). Filling volumes
supports:

  • run container from URL
  • inject binaries into arbitrary other images and then execute them
  • Git volumes and any other data driven volume
  • standard "wait for services container"

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Mar 30, 2016 via email

@smarterclayton
Copy link
Contributor Author

Opened a proposal under #23666, this is a prototype PR until that is approved.

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 31, 2016
@smarterclayton smarterclayton changed the title WIP - Adding init containers to pods WIP - Prototype init containers in pods Mar 31, 2016
@k8s-github-robot k8s-github-robot added release-note-label-needed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 31, 2016
@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 1, 2016
@smarterclayton smarterclayton force-pushed the init_containers branch 5 times, most recently from 7a1bc1b to a94f4d3 Compare April 6, 2016 02:06
@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 6, 2016
@bprashanth
Copy link
Contributor

@k8s-bot test this github issue: #25629

@smarterclayton
Copy link
Contributor Author

... Starting cluster in us-central1-f using provider gce
... calling verify-prereqs


Your current Cloud SDK version is: 109.0.0
Installing components from version: 109.0.0

+----------------------------------------------+
|     These components will be installed.      |
+-----------------------+------------+---------+
|          Name         |  Version   |   Size  |
+-----------------------+------------+---------+
| gcloud Alpha Commands | 2016.01.12 | < 1 MiB |
+-----------------------+------------+---------+

For the latest full release notes, please visit:
  https://cloud.google.com/sdk/release_notes

Do you want to continue (Y/n)?  
#============================================================#
#= Creating update staging area                             =#
#============================================================#
ERROR: gcloud crashed (OSError): [Errno 39] Directory not empty: '/usr/local/share/google/google-cloud-sdk.staging'

If you would like to report this issue, please run the following command:
  gcloud feedback
ERROR: gcloud crashed (IOError): [Errno 2] No such file or directory: '/usr/local/share/google/google-cloud-sdk/.install/bq-nix.snapshot.json'

If you would like to report this issue, please run the following command:
  gcloud feedback

@smarterclayton
Copy link
Contributor Author

Also

docker: error while loading shared libraries: libltdl.so.7: cannot open shared object file: No such file or directory
!!! Error in ./hack/update-api-reference-docs.sh:71
  'docker run ${user_flags} --rm -v "${TMP_IN_HOST}":/output:z -v "${SWAGGER_PATH}":/swagger-source:z gcr.io/google_containers/gen-swagger-docs:v5 "${SWAGGER_JSON_NAME}" "${REGISTER_FILE_URL}"' exited with status 127
Call stack:
  1: ./hack/update-api-reference-docs.sh:71 main(...)
Exiting with status 1
!!! Error in ./hack/../hack/verify-api-reference-docs.sh:34
  '"./hack/update-api-reference-docs.sh" "${OUTPUT_DIR}"' exited with status 1
Call stack:
  1: ./hack/../hack/verify-api-reference-docs.sh:34 main(...)
Exiting with status 1
FAILED   ./hack/../hack/verify-api-reference-docs.sh    1s

All sorts of goodness this weekend.

@smarterclayton
Copy link
Contributor Author

@k8s-bot test this issue #25635

@smarterclayton smarterclayton removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 16, 2016
@smarterclayton
Copy link
Contributor Author

Found a failure mode (caught by the e2e test checking the message) - init containers are only ready when they have a recorded container that is terminated with exit code 0. Was introduced in the status rebase (the new code was incorrect, the old code was correct). PTAL.

@smarterclayton
Copy link
Contributor Author

smarterclayton commented May 16, 2016

@k8s-bot test this github issue: #IGNORE (bot got stuck last night)

@smarterclayton
Copy link
Contributor Author

@k8s-bot test this issue #IGNORE (node e2e never started, looks like job was terminated early).

@bprashanth
Copy link
Contributor

I guess kubelet though it was ready because of the absence of a probe in the previous version. LGTM, feel free to re-apply label when you've squashed the last fix commit.

@smarterclayton smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2016
@k8s-bot
Copy link

k8s-bot commented May 18, 2016

GCE e2e build/test passed for commit 2a53330.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented May 18, 2016

GCE e2e build/test passed for commit 2a53330.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit bf4f841 into kubernetes:master May 18, 2016
@bprashanth
Copy link
Contributor

Congratulations to all!

@smarterclayton
Copy link
Contributor Author

I need to update the proposal and merge it.

On Wed, May 18, 2016 at 9:02 AM, Prashanth B notifications@github.com
wrote:

Congratulations to all!


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23567 (comment)

@bprashanth
Copy link
Contributor

Best way to get agreement on any proposal

@smarterclayton
Copy link
Contributor Author

Shipping code wins.

On Wed, May 18, 2016 at 12:55 PM, Prashanth B notifications@github.com
wrote:

Best way to get agreement on any proposal


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23567 (comment)

k8s-github-robot pushed a commit that referenced this pull request Jun 18, 2016
Automatic merge from submit-queue

Proposal for implementing init containers

Addresses #1589. Implemented in #23567. Docs in kubernetes/website#679

```release-note
Init containers enable pod authors to perform tasks before their normal containers start. Each init container is started in order, and failing containers will prevent the application from starting.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.