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

WIP [SDK] - Proposal to split Helm/Ansible on their repos #44

Closed
wants to merge 2 commits into from
Closed

WIP [SDK] - Proposal to split Helm/Ansible on their repos #44

wants to merge 2 commits into from

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Aug 30, 2020

This proposal has tables and images. I'd recommend you use its preview to check out.

TODO

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Lots of work still to be done:

  • User stories not written correctly.
  • Some ambiguity in language, specifically about the word "types". Project types? Go types? Kinds?
  • I don't see much discussion of how the release process will carry over to split repos either.
  • Docs changes are out of scope, but there is still the implementation detail of updating all docs to point to the correct repos.

enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
@camilamacedo86 camilamacedo86 changed the title Proposal to split Helm/Ansible on their repos [SDK] - Proposal to split Helm/Ansible on their repos Sep 6, 2020
@camilamacedo86 camilamacedo86 changed the title [SDK] - Proposal to split Helm/Ansible on their repos WIP - [SDK] - Proposal to split Helm/Ansible on their repos Sep 7, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2020
@camilamacedo86 camilamacedo86 changed the title WIP - [SDK] - Proposal to split Helm/Ansible on their repos [SDK] - Proposal to split Helm/Ansible on their repos Sep 7, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2020
@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Sep 7, 2020

Hi @estroz,

Really thank you for your time and review. it was exactly the kind of feedback that I was looking for to make this proposal more detailed and mature. See that I address more details as simplified it. Fell free to re-check.

- Improve the maintainability, readability and reusability by adopting concepts such as [separation of concerns](https://en.wikipedia.org/wiki/Separation_of_concerns) and [Single Responsibility principle](https://en.wikipedia.org/wiki/Single-responsibility_principle).
- Facilitate the development process and encourage more contributions
- Allow Lifecycle releases per type
- Minimize the CI processing time
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @estroz, IMO the time to process the CI has been a big problem for us. So, I agree with you that it is not a Goal. However, I think that it is a Motivation for we put it in place.

As a longer-term option solution, after the second phase of plugins are implemented, and its design be consolidated, we might be able to provide from upstream a base config plugin which could scaffold the `config/` directory as a common Makefile for any Operator Type be able to re-use it and perform its customizations as scaffold its specific languages type instead of Go. SDK also could provide as lib its plugins for customizations to be done on top of it.

This solution could be helpful to provide alternatives to developers quickly built their Operator Types such as to support [Argo](https://github.com/operator-framework/operator-sdk/issues/2497) and to allow the common needs to be re-used and be easily support new Operator Types such as [Hybrid Operators](https://github.com/operator-framework/operator-sdk/issues/670) or with other languages see [Meta: Operator SDK's in other languages #2745](https://github.com/operator-framework/operator-sdk/issues/2745), so that we could easily join these new Operator Types as incubator projects and define as pre-requirement following the same standards and workflows to work with the SDK features. However, it is definitely out of the scope of this EP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @estroz,

I tried to clarify my ideas here. Besides these needs are not a Go of this EP I think it is important we have them in mind since they are ultimate goals for the projects. Otherwise, we might take decisions that will be harder to reach them in the future.

- Operator Types (`helm-operator` and `ansible-operator`) should contains tests that will cover its libs and the project scaffold by them.

- SDK CLI tool (`operator-sdk`) should contain the test required to cover the CLI commands and e2e tests which will cover the specific customizations made on top of helm/ansible plugins for example olm integrations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HI @estroz,

I tried to put here what in my mind should be covered on the tests per project.

Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

Is there a concern that by separating these, it can encourage feature skew between the types of operators? I wonder if there is some mitigation that we can put in place to make sure we don't do that?

enhancements/split_helm_ansible_into_their_repos.md Outdated Show resolved Hide resolved
@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Sep 16, 2020

Hi @shawn-hurley,

Really thank you for your input here.

Is there a concern that by separating these, it can encourage feature skew between the types of operators? I wonder if there is some mitigation that we can put in place to make sure we don't do that?

`outdated reply` It is one of the motivations of the `User Storie: Make upstream plugin Boilerplates public`.~~ The behaviours and configs which are common for all are defined in the config dir and in the Makefile. The idea here is to use directly the KB(upstream) code which also would avoid the effort to get its changes as well. And then, as a follow up we might able to have a Lib as described in `Alternatives: SDK Operator Build Lib`.

However, as we have not the plugin phase 2 implemented and defined as other points shows that would be easy we try to reduce the complexities and then, try to address the needs step by step. (first split with the code and remove the duplications by centralizing it in upstream, then after the plugin phase 2 get done we might have a common config base plugin responsable to scaffold the config dir and Makefile which will be customized by the Operator Types and then we maybe can improve it to address other requirements by offering a lib with helpers for other Operator Types)

I tried to supplement the doc please let me know if has something that would need further detail to clarify its motivations.
Also, please feel free to share your ideas and insights as well.

The prosed solution in kubernetes-sigs/kubebuilder#2015 can solve this problem. We need to update this EP. c/c @jmrodri

@camilamacedo86 camilamacedo86 changed the title [SDK] - Proposal to split Helm/Ansible on their repos WIP"[SDK] - Proposal to split Helm/Ansible on their repos Feb 16, 2021
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 16, 2021
@camilamacedo86 camilamacedo86 changed the title WIP"[SDK] - Proposal to split Helm/Ansible on their repos WIP [SDK] - Proposal to split Helm/Ansible on their repos Feb 16, 2021
@camilamacedo86
Copy link
Contributor Author

c/c @fabianvf @asmacdo
I am closing this one because it is outdated.
But I want to ping you in case that can help you out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants