Skip to content

Conversation

@maierlars
Copy link
Contributor

Randomize root password or use provided secret.
This is a breaking change.

@maierlars maierlars self-assigned this Jan 14, 2019
@maierlars maierlars requested a review from neunhoef January 14, 2019 13:31
@ghost ghost added the 2 - Working label Jan 14, 2019
Copy link
Member

@neunhoef neunhoef left a comment

Choose a reason for hiding this comment

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

LGTM

@neunhoef
Copy link
Member

There are still open questions:
(1) This is a breaking change, since all documentation is wrong now.
(2) What happens in an upgrade case if the operator is upgraded?
(3) How should the field in the spec be called and what is its semantics?

@neunhoef
Copy link
Member

Documentation missing.

Copy link
Contributor

@ewoutp ewoutp left a comment

Choose a reason for hiding this comment

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

I don't like the breaking change.
Let's change that.

Other than that comments are missing, some naming issues and lack of documentation.

Copy link
Contributor

@ewoutp ewoutp left a comment

Choose a reason for hiding this comment

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

  1. Couple of minor issues.
  2. K8s resource naming issue is worrying. At least fix that.

After that LGTM

Copy link
Contributor

@ewoutp ewoutp left a comment

Choose a reason for hiding this comment

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

Couple of issues, then LGTM

Copy link
Member

@neunhoef neunhoef left a comment

Choose a reason for hiding this comment

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

LGTM

@neunhoef neunhoef merged commit 0e2689d into master Mar 25, 2019
@neunhoef neunhoef deleted the feature/bootstrap-root-pwd branch March 25, 2019 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants