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

Update documentation for RBAC cluster#175

Closed
laghoule wants to merge 0 commit into
coreos:masterfrom
laghoule:master
Closed

Update documentation for RBAC cluster#175
laghoule wants to merge 0 commit into
coreos:masterfrom
laghoule:master

Conversation

@laghoule

Copy link
Copy Markdown
Contributor
  • Update documentation to include namespace creation
  • Add instruction for RBAC enabled cluster

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

2 similar comments
@gojihotsauce

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@sdemos

sdemos commented Feb 15, 2018

Copy link
Copy Markdown
Contributor

Thanks for the pr! If you don't mind, can you squash all the commits into one?

I'm not super familiar with RBAC, can you take a look at this and make sure it's all good @dghubble?

Comment thread README.md Outdated

For RBAC enabled cluster:
```
kubectl create -f examples/namespace.yaml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can also just simplify this to kubectl apply -f examples -R.

Comment thread README.md Outdated

Create the `update-operator` deployment and `update-agent` daemonset.

For non-RBAC cluster:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't really have to encourage those still running non-RBAC clusters. Who are these people? (half joking)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed

@dghubble

Copy link
Copy Markdown
Contributor

Contents are fine since its just instructing users to apply the examples we already supply.

@gojihotsauce

Copy link
Copy Markdown

Can one of the admins verify this patch?

8 similar comments
@gojihotsauce

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@sdemos

sdemos commented Mar 27, 2018

Copy link
Copy Markdown
Contributor

sorry about the bot spam. we are having some issues with our jenkins. also sorry for taking so long on this! the document update looks good to me. do you mind squashing all the commits into one? when you do that I'll go ahead and merge it.

@coreosbot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@sdemos

sdemos commented Mar 27, 2018

Copy link
Copy Markdown
Contributor

On a second pass, I noticed that we actually have a couple of example manifests in that directory which shouldn't be deployed normally, under the reboot-annotations directory. We should probably re-organize the examples directory a little bit to make it easier to deploy with the default configuration.

I'm thinking probably something like -

examples
├── deploy
│   ├── 00-namespace.yaml
│   ├── rbac
│   │   ├── cluster-role-binding.yaml
│   │   └── cluster-role.yaml
│   ├── update-agent.yaml
│   └── update-operator.yaml
└── reboot-annotations
    ├── after-reboot-daemonset.yaml
    └── before-reboot-daemonset.yaml

and then updating the documentation to reflect that, and updating the build/bump-release script to make sure the version templated files are still correct.

@laghoule I know that that's a little more than you originally signed up for, so I understand if you don't want to do that. I can take it on instead if you prefer. Just let me know.

@laghoule

Copy link
Copy Markdown
Contributor Author

@sdemos I will do it.
I will follow your examples tree, it seem logical.

@sdemos sdemos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found a couple more references to the old examples location in doc/development.md, and it looks like the example before and after reboot daemonset locations in doc/before-after-reboot-checks.md didn't get updated correctly a while ago on lines 50 and 51.

Other than that, the changes look great. Just go ahead and squash the commits into probably two commits, one with the example restructuring, and one with the document changes.

@laghoule

Copy link
Copy Markdown
Contributor Author

Need to read the git man page for the squashing. Will do it tomorrow.

@laghoule

laghoule commented Apr 6, 2018

Copy link
Copy Markdown
Contributor Author

@sdemos you need something else?

@sdemos

sdemos commented Apr 6, 2018

Copy link
Copy Markdown
Contributor

The changes look good, and those commits are done well, but the history is still a little messy. In the interest of getting these changes merged in, I'm going to go ahead and massage the git history a little so it better reflects the changes.

@sdemos

sdemos commented Apr 6, 2018

Copy link
Copy Markdown
Contributor

sorry, sorry, I messed up the git command. Your changes will be back shortly

@sdemos

sdemos commented Apr 6, 2018

Copy link
Copy Markdown
Contributor

Okay, so, I made a mistake with the git push syntax and accidentally closed this pr and simultaneously cut off my ability to fix it here. I opened a new pr with the commits that were supposed to end up here (#178) which also details the mistake for those who are curious.

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.

5 participants