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

docs: fix broken links (CI) #3514

Merged
merged 3 commits into from
Jul 24, 2020

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Jul 24, 2020

Description of the change:
docs: fix broken links (CI)

Motivation for the change:
fix CI and 1.0.0

@@ -231,38 +238,7 @@ func contains(list []string, s string) bool {
During the lifecycle of an operator it's possible that there may be more than 1 instance running at any given time e.g when rolling out an upgrade for the operator.
In such a scenario it is necessary to avoid contention between multiple operator instances via leader election so that only one leader instance handles the reconciliation while the other instances are inactive but ready to take over when the leader steps down.

There are two different leader election implementations to choose from, each with its own tradeoff.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just removed the part of the sdk impl and all the rest of the texts were kept in the same way.

the following link was comment for do no broke the CI
this impl probably will came from operator-lib
however, we also need to check if we would not like to just let them use
https://github.com/kubernetes/kubernetes/pull/92717/files
[godoc-conditions]: https://godoc.org/github.com/operator-framework/operator-sdk/pkg/status#Conditions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this one is falling now. I am not sure if we will still providing or not the helper in via operator-lib so just comment and raised : #3515

@camilamacedo86 camilamacedo86 added this to the v1.0.0 milestone Jul 24, 2020
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.

I don't see any reason to remove the docs about leader for life. We just need to update the link to point to operator-lib

@camilamacedo86 camilamacedo86 changed the title docs: remove leader sdk implementation that is no longer supported - … docs: fix broken links (CI) Jul 24, 2020
Comment on lines 31 to 35
<!-- todo(camilamacedo)
the following link was comment for do no broke the CI
this impl probably will came from operator-lib
however, we also need to check if we would not like to just let them use
https://github.com/kubernetes/kubernetes/pull/92717/files
Copy link
Member

Choose a reason for hiding this comment

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

pkg/status was also moved to operator-lib. Let's just update that link as well.

And I don't think this is the appropriate place for a comment about the choice of implementations to use.

I think a better choice would be to open an issue in operator-lib asking whether it makes sense to deprecate status and point users to k8s.io/apimachinery's implementation.

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.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2020
@joelanford joelanford merged commit b158b7e into operator-framework:master Jul 24, 2020
@camilamacedo86 camilamacedo86 deleted the remove-leader branch July 24, 2020 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants