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

Prepare pkg/ansible and pkg/helm for v1.0.0 release #1186

Closed
joelanford opened this issue Mar 7, 2019 · 6 comments
Closed

Prepare pkg/ansible and pkg/helm for v1.0.0 release #1186

joelanford opened this issue Mar 7, 2019 · 6 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion

Comments

@joelanford
Copy link
Member

Problem
We currently export most of the internals of the Ansible and Helm operators, which is technically unnecessary, since only the ansible.Run() and helm.Run() entrypoints and their flag parameters are used in operator-sdk run and in scaffolds created by operator-sdk migrate.

Leaving pkg/ansible and pkg/helm in their current states will make it difficult to support new features after we release v1.0.0 and are constrained by the backwards-compatible guarantees of a major release.

However we may want to expose more than just the Run() functions in hybrid operator use cases, so we need to discuss what else we should leave exported in our public API from pkg/ansible and pkg/helm.

/cc @shawn-hurley

@jmrodri
Copy link
Member

jmrodri commented Mar 21, 2019

I'd like to understand more about this. What is considered a public API? how are is the API expected to be used?

@joelanford
Copy link
Member Author

What is considered a public API?

In Go, an identifier is public if it is exported (begins with a capital letter) and is in a package tree that doesn't contain an internal directory in its path.

IMO, I would consider all public identifiers to be part of our public API (since that's what the go toolchain considers public) and subject to our backwards compatibility guarantees.

There's some discussion about this starting here, where the consensus is that it is reasonable to expect consumers of operator-sdk not to use exported identifiers from packages in the cmd/ tree. I personally think that's debatable since the go toolchain will happily document (go doc) and allow external libraries to import those packages (go build), thus making them de facto public.

At the very least, we should probably document what we consider our public API to be, but the more we diverge from what the go toolchain considers it to be, the more likely we are to have users consuming APIs that we don't consider public (and the more likely we are to break those users).

In the case of pkg/ansible and pkg/helm, I don't think there's any question that they're public, since they're in the pkg/ tree, which is the conventional location for sharable library code.

how are is the API expected to be used?

We primarily expect our APIs to be used by operator authors using the SDK to create and manage their projects. Many of our exported libraries are used by the scaffolded operators we create with operator-sdk new

We expect the ansible and helm APIs to be used after SDK users run operator-sdk migrate. The goal of this issue is to determine which of the ansible and helm APIs are actually required to enable a robust experience for operator-sdk migrate workflows. Any that don't meet that criteria should be unexported to give us more flexibility to make changes in the future.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 19, 2019
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 19, 2019
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs discussion
Projects
None yet
Development

No branches or pull requests

4 participants