Skip to content

install: Introduce the Identity controller#2526

Merged
olix0r merged 9 commits intomasterfrom
ver/identity-install
Mar 20, 2019
Merged

install: Introduce the Identity controller#2526
olix0r merged 9 commits intomasterfrom
ver/identity-install

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented Mar 19, 2019

#2521 introduces an "Identity" controller, but there is no way to include it in linkerd installation.

This change alters the install flow as follows:

  • An Identity service is always installed;
  • Issuer credentials may be specified via the CLI;
  • If no Issuer credentials are provided, they are generated each time install is called.
  • It's possible to override the credential generation logic, especially for tests;
  • Proxies are NOT configured to use the identity service.

@olix0r olix0r self-assigned this Mar 19, 2019
@olix0r olix0r changed the base branch from ver/identity-service to master March 19, 2019 21:25
#2521 introduces an "Identity"
controller, but there is no way to include it in linkerd installation.

This change alters the `install` flow as follows:
- An Identity service is _always_ installed;
- Issuer credentials may be specified via the CLI;
- If no Issuer credentials are provided, they are generated each time `install` is called.
- Proxies are NOT configured to use the identity service.
- It's possible to override the credential generation logic---especially
  for tests---via install options that can be configured via the CLI.
@olix0r olix0r force-pushed the ver/identity-install branch from b409040 to 18368cc Compare March 19, 2019 22:19
@olix0r olix0r marked this pull request as ready for review March 19, 2019 22:19
@olix0r olix0r requested review from alpeb, ihcsim and klingerf March 19, 2019 22:20
Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

Looking good. I had constructed most of my comments based on the previous diff, so a few of these may be outdated. Sorry about that. Will give it a re-review once you've addressed the rest.

Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Great, updates look good to me.

We shouldn't do this as part of this branch, but I'm starting a small list of tasks that we should complete in order to treat the new identity pod as a fully fledged member of the control plane, as follows:

  • Update linkerd check to start verifying that the new pod is up and running
  • Update the web UI to display the health of the pod in the table on the Service Mesh page
  • Update the install integration test to validate that the identity pod is running and has not been restarted

There are probably a few more places it needs to be added as well.

Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few questions.

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