CORS-4161: Add GCP Service Endpoint overrides #896
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@barbacbd: This pull request references CORS-4161 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@barbacbd: This pull request references CORS-4161 which is a valid jira issue. In 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cc @patrickdillon |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #896 +/- ##
==========================================
- Coverage 47.00% 46.50% -0.50%
==========================================
Files 97 98 +1
Lines 11917 12053 +136
==========================================
+ Hits 5601 5605 +4
- Misses 5698 5830 +132
Partials 618 618
🚀 New features to boost your workflow:
|
|
/retest-required |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
/assign |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: barbacbd The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pkg/util/featuregate.go
Outdated
| enabledFeatureGates := map[configv1.FeatureGateName]bool{} | ||
| for _, fg := range featureGates { | ||
| enabledFeatureGates[fg] = currentFeatureGates.Enabled(fg) | ||
| } | ||
| return enabledFeatureGates, nil |
There was a problem hiding this comment.
Instead, let's have this function GetCurrentFeatureGates and return the currentFeatureGates from line 47. This will change the behavior in pkg/operator/controller.go as mentioned in the correlated comment.
return featureGateAccessor.CurrentFeatureGates()
pkg/operator/controller.go
Outdated
| enabledFeatures, err := util.GetEnabledFeatureGates([]configv1.FeatureGateName{features.FeatureGateGCPCustomAPIEndpoints}) | ||
| if err != nil { | ||
| return fmt.Errorf("error getting enabled feature gates: %v", err) | ||
| } | ||
| gcpCustomEndpointsEnabled, _ := enabledFeatures[features.FeatureGateGCPCustomAPIEndpoints] |
There was a problem hiding this comment.
In conjunction with the comment in pkg/util/featuregate.go, modify this to get currentFeatureGates, and then check for gcpCustomEndpointsEnabled separately.
currentFeatureGates, err := util.GetCurrentFeatureGates()
if err != nil {
return err
}
That block should be included in the prior commit so it remains after the featuregate is removed. And then for this specific feature gate, you would add
gcpCustomEndpointsEnabled := currentFeatureGates.Enabled(features.FeatureGateGCPCustomAPIEndpoints)
pkg/util/featuregate.go
Outdated
| currentFeatureGates, err := featureGateAccessor.CurrentFeatureGates() | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
nit: this feels replicated from the case above.
pkg/util/featuregate.go
Outdated
| desiredVersion := "unknown" // TODO: config.OperatorReleaseVersion <<< passed or read into the operator | ||
| missingVersion := "0.0.1-snapshot" |
There was a problem hiding this comment.
I don't think CCO properly handles versioning. We might have to set desiredVersion equal to missingVersion here to make this work, which looks like it will then read the version from CVO.
barbacbd
force-pushed
the
CORS-4161
branch
2 times, most recently
from
3a2dc90 to
91b7741
Compare
|
Currently this is on hold for multiple reasons: |
barbacbd
force-pushed
the
CORS-4161
branch
2 times, most recently
from
91b7741 to
a050fca
Compare
|
@barbacbd: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
pkg/cmd/provisioning: ** Temporarily updated to pass nil for service endpoints. This is a placeholder while ccoctl does not support user supplied service endpoints. pkg/gcp/actuator: ** Actuator will get the infrastructure object and pass the endpoints during client creation. pkg/operator/secretannotator/gcp: ** Reconciler will get the infrastructure object and pass the endpoints during client creation. pkg/gcp/client.go: ** Client will now accept endpoints when creating the service cleints (ex: compute).
…CP custom Endpoint feature gate. Note: This commit is temporary and should be removed upon feature promotion.
pkg/cmd/provisioning:
** Temporarily updated to pass nil for service endpoints. This is a placeholder while ccoctl does not support user supplied service endpoints.
pkg/gcp/actuator:
** Actuator will get the infrastructure object and pass the endpoints during client creation.
pkg/operator/secretannotator/gcp:
** Reconciler will get the infrastructure object and pass the endpoints during client creation.
pkg/gcp/client.go:
** Client will now accept endpoints when creating the service cleints (ex: compute).