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

Fix typos and add a paragraph for initializers doc #4369

Merged
merged 2 commits into from
Aug 1, 2017
Merged

Fix typos and add a paragraph for initializers doc #4369

merged 2 commits into from
Aug 1, 2017

Conversation

ahmetb
Copy link
Member

@ahmetb ahmetb commented Jul 14, 2017

  • Fixed a few consistency issues and typos in the doc
  • Also fixed an username typo in assignees
  • Added a paragraph explaining how the example initializerconfiguration will
    be applied once it is created.

/assign @chenopis


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 14, 2017
@kubernetes kubernetes deleted a comment from k8s-ci-robot Jul 14, 2017
@ahmetb
Copy link
Member Author

ahmetb commented Jul 14, 2017

/assign @caesarxuchao

- Fixed a few consistency issues and typos in the doc
- Also fixed an username typo in assignees
- Added a paragraph explaining how the example initializerconfiguration will
  be applied once it is created.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
@@ -126,6 +126,11 @@ initializers:
- pods
```

Once this `initializerConfiguration` named `example-config` is created, the
Copy link
Member

Choose a reason for hiding this comment

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

Could you move it to after line 137, so it will read like:

"After you create the initializerConfiguration, the system will take a few seconds to honor the new configuration. Then, "podimage.example.com" will be appended to the metadata.initializers.pending field of newly created pods. You should already have a ready "podimage" initializer controller that handles pods whose metadata.initializers.pending[0].name="podimage.example.com". Otherwise the pods will stuck uninitialized."

BTW, there is another occurrence of metadata.initializers[0] earlier in the doc, could you replace it with metadata.initializers.pending[0].name?

Copy link
Member Author

Choose a reason for hiding this comment

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

@caesarxuchao there were about 4 other occurrences of metadata.initializers that doesn't mention .pending. I changed them all. PTAL again.

@caesarxuchao
Copy link
Member

Thanks. A comment, otherwise lgtm.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
should already have a ready "podimage" initializer controller that handles pods
whose `metadata.initializers.pending[0].name="podimage.example.com"`. Otherwise
the pods will stuck uninitialized.

Make sure that all expansions of the `<apiGroup, apiVersions, resources>` tuple
Copy link
Member

Choose a reason for hiding this comment

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

I feel this paragraph should go before "After you create ...", since it's talking about the best-practice of InitializerConfiguration. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it here because explaining what this InitializerConfiguration does seemed more important. Can move if necessary, but I think it reads fine.

Copy link
Member

Choose a reason for hiding this comment

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

It's ok. I don't feel strongly about it.

@caesarxuchao
Copy link
Member

/lgtm

@ahmetb
Copy link
Member Author

ahmetb commented Jul 25, 2017

/assign @zacharysarah
/unassign @chenopis

@chenopis chenopis self-assigned this Aug 1, 2017
@chenopis chenopis merged commit acef396 into kubernetes:master Aug 1, 2017
jesscodez pushed a commit that referenced this pull request Sep 22, 2017
* Fix typos and add a paragraph for initializers doc

- Fixed a few consistency issues and typos in the doc
- Also fixed an username typo in assignees
- Added a paragraph explaining how the example initializerconfiguration will
  be applied once it is created.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>

* Add .pending to metadata.initializers, re-wording

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants