Skip to content

Conversation

@osherdp
Copy link

@osherdp osherdp commented Dec 1, 2020

This PR adds annotation for the single-node-production-edge cluster profile. There's a growing requirement from several customers to enable creation of single-node (not high-available) Openshift clusters.
In stage one (following openshift/enhancements#504) there should be no implication on components logic.
In the next stage, the component's behavior will match a non high-availability profile if the customer is specifically interested in one.
This PR is separate from the 'single-node-developer' work, which will implement a different behavior and is currently on another stage of implementation.

For more info, please refer to the enhancement link and participate in the discussion.

@stlaz
Copy link
Contributor

stlaz commented Dec 1, 2020

Please explain in details of what the annotation actually does in the commit message, link to an enhancement won't do for anyone who wants to quickly understand why the annotation's there.

I haven't read the enhancement and I do not know the consequences of this annotation. Single-node cluster sounds contradictive. Does that mean the operator's code will need to change? Who is going to implement the necessary changes?

How is this different from the other "single-node" PR? Why does it have to be 2 PRs instead of one with two commits?

@stlaz
Copy link
Contributor

stlaz commented Dec 1, 2020

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 1, 2020
@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

@stlaz updated description, please take a look

@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@stlaz
Copy link
Contributor

stlaz commented Dec 2, 2020

@osherdp Awesome, please also remove the JIRA reference from the commit message.

I am also concerned about

In the next stage, the component's behavior will match a non high-availability profile if the customer is specifically interested in one.

What does it mean "will match a non high-availability profile"? CAO makes assumptions about high-availability of kube-apiserver as it may restart it as a reaction to user configuration. What happens to the platform when there is no kube-apiserver? How do you make this component "match a non high-availability profile"? Is that something that will require changes to the component? Who will implement them?

@osherdp osherdp force-pushed the enhancement/single-node-annotation branch from e8a694d to d181e7d Compare December 2, 2020 09:20
@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

@stlaz not only CAO, but many components are doing restarts. We have MCO which also does a complete node reboot and we do know to handle that
I assume most changes are pretty trivial and can be made by our team (edge), some edge cases will need help from the responsible maintainers of the component

@stlaz
Copy link
Contributor

stlaz commented Dec 2, 2020

/hold cancel
/lgtm
@osherdp ok, I think that addresses all my concerns for now. Thanks.

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: osherdp, stlaz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 2, 2020
@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

3 similar comments
@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

2 similar comments
@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 2, 2020

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@osherdp
Copy link
Author

osherdp commented Dec 3, 2020

/retest

2 similar comments
@osherdp
Copy link
Author

osherdp commented Dec 3, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 3, 2020

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@osherdp osherdp force-pushed the enhancement/single-node-annotation branch from d181e7d to fbe5bbf Compare December 3, 2020 11:34
@openshift-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2020
@osherdp
Copy link
Author

osherdp commented Dec 6, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 6, 2020

/hold for openshift/api#813

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2020
@osherdp osherdp changed the title MGMT-3105: add 'single-node-production-edge' annotations to CVO manifests add 'single-node-production-edge' annotations to CVO manifests. Dec 7, 2020
@stlaz
Copy link
Contributor

stlaz commented Dec 9, 2020

/hold
Merge branch 'master' into enhancement/single-node-annotation <- remove this commit

@osherdp osherdp force-pushed the enhancement/single-node-annotation branch from a8a4963 to d4bfe6c Compare December 9, 2020 09:38
@osherdp
Copy link
Author

osherdp commented Dec 9, 2020

@stlaz done
keep on hold until they'll merge api's PR

@osherdp
Copy link
Author

osherdp commented Dec 9, 2020

/retest

@osherdp
Copy link
Author

osherdp commented Dec 15, 2020

Shifting strategy based on openshift/enhancements#560

@osherdp osherdp closed this Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants