docs/dev/operators: Document a dynamic-object approach #59
Conversation
Also: * Backticked 'image-references' and 'sed' instances and fixed a doubled space. * Downcased "See" -> "see" in a parenthetical, because it's not a new sentence. * Make "below" a link, since that's easier to follow than just the bare word. * Changed "dir" -> "directory", because there's no need to abbreviate there.
It feels like the wording wanted this to be an enumerated list, but the text landed without that markup in e046cd7 (docs/dev: Second level operator FAQ, 2018-10-08, openshift#34).
This content is descended from openshift/installer@9c8a77dd (docs/dev: Doc on how to integrate an operator, 2018-09-28, openshift/installer#377), but handling dynamic configuration is a generic issue, so the docs should live in the generic CVO repository. This will make these docs more discoverable. It will also allow the installer docs to focus on installer-specific issues for the small subset of operators that need direct installer bindings.
openshift-ci-robot
added
approved
That approach should be documented in the CVO itself, since it's not installer-specific and moving it gets the docs and implementation for that approach into the same repository. I've filed [1] to land dynamic-object docs in the CVO repo (based on some of the content I'm removing here). Naming files, etc. are already covered by the existing CVO documentation. [1]: openshift/cluster-version-operator#59
rajatchopra
left a comment
rajatchopra
left a comment
There was a problem hiding this comment.
/lgtm
Minor nice to have about an inline example. All good nevertheless.
| 2. Generates the dynamic configuration based on the dependencies. | ||
| 3. Pushes the generated configuration into the cluster, where future operator runs and other consumers will be able to find it. | ||
|
|
||
| For an example, see [the machine-API operator's `getOperatorConfig`][getOperatorConfig], although they don't need the "push the generated configuration into the cluster" step. |
There was a problem hiding this comment.
Maybe we should inline the example in a code block here.
There was a problem hiding this comment.
Maybe we should inline the example in a code block here.
How much of the code? e24bc29 currently links to the main portion of this, and I mention InClusterConfig further up but don't link to the operator's InClusterConfig call or client construction. Putting that all together into one example would be something like:
include (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"
)
func example() {
config, err := rest.InClusterConfig() // FIXME: error handling
client := kubernetes.NewForConfigOrDie(rest.AddUserAgent(config, "your-operator")).CoreV1().ConfigMaps("kube-system")
for {
config, err := getConfig(client) // FIXME: error handling
// FIXME: do whatever your operator does
}
}
func getConfig(client corev1.ConfigMapInterface) (FIXMESomeType, error) {
clusterConfig, err := client.Get("cluster-config-v1", metav1.GetOptions{}) // FIXME: error handling
return generateYourConfig(clusterConfig)
}That's a lot of FIXMEs for things I've skipped, and it's still fairly involved. Is it particularly helpful when we're already linking folks to the not-much-more-complicated real-world machine-API operator implementation?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rajatchopra, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@rajatchopra: changing LGTM is restricted to assignees, and only openshift/cluster-version-operator repo collaborators may be assigned issues. DetailsIn response to this:
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. |
|
/retest |
|
/retest |
1 similar comment
|
/retest |
| ``` | ||
|
|
||
| Ensure your image is published into the cluster release tag by ci-operator | ||
| Wait for a new release payload to be created (usually once you push to master in your operator). |
There was a problem hiding this comment.
nit: will be nice to add a period - tag by ci-operator. Wait for a new release...
|
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci-robot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci-robot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
openshift-ci
bot
added
the
needs-rebase
|
@wking: PR needs rebase. DetailsInstructions 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. |
|
@openshift-bot: Closed this PR. DetailsIn response to this:
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. |
6 participants
This content is descended from openshift/installer@9c8a77dd (openshift/installer#377), but handling dynamic configuration is a generic issue, so the docs should live in the generic CVO repository. This will make these docs more discoverable. It will also allow the installer docs to focus on installer-specific issues for the small subset of operators that need direct installer bindings.
There are also two orthogonal copy-edit commits in this PR. I'm also fine pulling those out into a separate PR (or PRs), or with squashing them all into a single commit, if any of that would make review easier.