Skip to content

Add utility functions for FeatureGates#1208

Closed
adambkaplan wants to merge 1 commit intoopenshift:masterfrom
adambkaplan:feature-gate-enabled
Closed

Add utility functions for FeatureGates#1208
adambkaplan wants to merge 1 commit intoopenshift:masterfrom
adambkaplan:feature-gate-enabled

Conversation

@adambkaplan
Copy link
Copy Markdown
Contributor

@adambkaplan adambkaplan commented Sep 13, 2021 •
edited
Loading

  • Export the getFeaturesFromTheSpec function, renaming it
    GetEnabledAndDisabledFeatures and adding appropriate godoc. This
    will let library-go users determine the list of enabled and disabled
    features independently of the ObserveFeatureFlags function.
  • Add a general purpose function for checking if a feature gate has been
    enabled.
  • Implement unit tests for the new library functions.

@openshift-ci openshift-ci Bot requested review from deads2k and soltysh September 13, 2021 16:11
@adambkaplan adambkaplan mentioned this pull request Sep 13, 2021
Merged
@adambkaplan
Copy link
Copy Markdown
Contributor Author

adambkaplan commented Sep 13, 2021

/assign @mfojtik

/cc @deads2k @gabemontero

@openshift-ci openshift-ci Bot requested a review from gabemontero September 13, 2021 17:32
@adambkaplan
Copy link
Copy Markdown
Contributor Author

adambkaplan commented Sep 21, 2021

Potential usage in cluster-storage-operator: openshift/cluster-storage-operator#216

sttts
sttts reviewed Sep 22, 2021
View reviewed changes
Comment thread pkg/operator/configobserver/featuregates/observe_featuregates.go Outdated
func getFeaturesFromTheSpec(fg *configv1.FeatureGate) ([]string, []string, error) {
// GetEnabledAndDisabledFeatures returns a list of enabled and disabled features for the given `FeatureGate` instance.
// Returns an error if the object references an unknown feature set.
func GetEnabledAndDisabledFeatures(fg *configv1.FeatureGate) ([]string, []string, error) {
Copy link
Copy Markdown
Contributor

@sttts sttts Sep 22, 2021

Choose a reason for hiding this comment

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

I don't think configobservers are a good place to host any exported funcs beyond the observer.

Copy link
Copy Markdown
Contributor Author

@adambkaplan adambkaplan Sep 24, 2021

Choose a reason for hiding this comment

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

@sttts would a top level pkg/featuregates package be a better home for these functions?

@adambkaplan adambkaplan changed the title Export getFeaturesFromTheSpec Add utility functions for FeatureGates Nov 12, 2021
@adambkaplan
Copy link
Copy Markdown
Contributor Author

adambkaplan commented Nov 12, 2021

@sttts I've updated this PR to move the utility functions to the new pkg/config/featuregate package. PTAL.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 12, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: adambkaplan
To complete the pull request process, please assign deads2k after the PR has been reviewed.
You can assign the PR to them by writing /assign @deads2k in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@adambkaplan
Add utility functions for FeatureGates
1f01c49 
- Export the `getFeaturesFromTheSpec` function, renaming it
  `GetEnabledAndDisabledFeatures` and adding appropriate godoc. This
  will let library-go users determine the list of enabled and disabled
  features independently of the `ObserveFeatureFlags` function.
- Add a general purpose function for checking if a feature gate has been
  enabled.
- Implement unit tests for the new library functions.
@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Feb 10, 2022

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 openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 10, 2022
@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Mar 12, 2022

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 openshift-ci Bot 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 Mar 12, 2022
@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Apr 11, 2022

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 openshift-ci Bot closed this Apr 11, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 11, 2022

@openshift-bot: Closed this PR.

Details

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

Reviewers

@deads2k deads2k Awaiting requested review from deads2k

@soltysh soltysh Awaiting requested review from soltysh

@gabemontero gabemontero Awaiting requested review from gabemontero

1 more reviewer

@sttts sttts sttts left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mfojtik mfojtik

Labels

lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@adambkaplan @openshift-bot @sttts @mfojtik