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

Add code for building and running helm operator binary #110

Merged

Conversation

varshaprasad96
Copy link
Member

The following changes are made in this PR:
1. Add code for building and running helm operator binary
2. Move pkg/internal/controllerutil out of internal/ dir
3. Add separate helpers to read watches.yaml file for helm operator
4. Move diff.go and types.go from pkg/internal to internal/

@coveralls
Copy link

coveralls commented Oct 12, 2021

Pull Request Test Coverage Report for Build 1408668137

  • 173 of 1021 (16.94%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-27.4%) to 61.977%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/legacy/helm/types/types.go 31 43 72.09%
internal/legacy/watches/watches.go 68 87 78.16%
internal/legacy/helm/client/client.go 8 93 8.6%
internal/legacy/release/manager_factory.go 0 106 0.0%
internal/legacy/controller/controller.go 7 119 5.88%
internal/legacy/release/manager.go 43 254 16.93%
internal/legacy/controller/reconcile.go 13 316 4.11%
Totals Coverage Status
Change from base Build 1368807916: -27.4%
Covered Lines: 1674
Relevant Lines: 2701

💛 - Coveralls

Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

As I understand it, all of this code is being moved here so that:

  1. We can slim down SDK repo and remove plugin implementations
  2. So that we can continue building the existing implementation of the bae image.

Is that right?

I'm wondering if we can dump everything from SDK into internal/legacy/* so that we a) keep it all isolated in one place to make it easier to eventually remove and b) don't pollute the new implementation's libraries.

Lastly, what's the plan for versioning this repo if its going to contain both old and new implementations side by side?

pkg/sdk/controllerutil/controllerutil.go Outdated Show resolved Hide resolved
@varshaprasad96 varshaprasad96 force-pushed the add/helm-lib-code branch 2 times, most recently from fa48880 to d1f2fa8 Compare October 15, 2021 17:05
@varshaprasad96 varshaprasad96 force-pushed the add/helm-lib-code branch 2 times, most recently from a771ec4 to 2f04ded Compare October 15, 2021 18:32
@varshaprasad96 varshaprasad96 changed the title Add helm library code Add code for building and running helm operator binary Oct 21, 2021
@fabianvf
Copy link
Member

I'm wondering if we can dump everything from SDK into internal/legacy/* so that we a) keep it all isolated in one place to make it easier to eventually remove and b) don't pollute the new implementation's libraries.

I think we should probably just drop the old implementation ASAP

@varshaprasad96
Copy link
Member Author

I'm wondering if we can dump everything from SDK into internal/legacy/* so that we a) keep it all isolated in one place to make it easier to eventually remove and b) don't pollute the new implementation's libraries.

I think we should probably just drop the old implementation ASAP

The PR has both the implementations merged. We have both helm and hybrid binaries built here. Since we would still continue to support helm binary, I am not moving this to the legacy/ dir (for now at least). The helpers between helm and hybrid which could be the same are consolidated already.

The following changes are made in this PR:
1. Add code for building and running helm operator binary
2. Move pkg/internal/controllerutil out of internal/ dir
3. Add separate helpers to read watches.yaml file for helm operator
4. Move `diff.go` and `types.go` from `pkg/internal` to `internal/`
Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
@varshaprasad96
Copy link
Member Author

@joelanford @fabianvf I have moved helm related code from SDK to internal/legacy. The next follow up step would be to remove this - which (may) require a breaking change in hybrid (or helm).

pkg/watches/helm/watches.go Outdated Show resolved Hide resolved
@varshaprasad96 varshaprasad96 merged commit 1e7e868 into operator-framework:main Nov 2, 2021
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.

None yet

4 participants