Skip to content

Conversation

@a-dsouza
Copy link
Contributor

@a-dsouza a-dsouza commented Apr 5, 2023

What this PR does / why we need it: Backporting the following v1beta1 support PRs for v4.11 control planes:

This is part of work preparing for the eventual deprecation of v1alpha1.

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

csrwng and others added 7 commits March 16, 2023 17:26
Adds code to convert from v1alpha1 to v1beta1
Updates code to use the new version
…onditionalUpdates

So that folks driving a HostedCluster have a way to steer their
update-recommendation engine.

The spec controls flow into external systems to HostedCluster.
HostedCluster passes them along to HostedControlPlane.
HostedControlPlane passes them along to the hosted ClusterVersion
resource.  And then the status data flows back up from ClusterVersion
to HostedControlPlane to HostedCluster for folks making update
decisions.

An alternative approach to managing hosted-cluster updates would be:

* Teaching the cluster-version operator to be HyperShift aware, with a
  separate kubeconfig for the management- and hosted-cluster control
  planes.
* Asking the CVO to automatically update whenever the update service
  recommeneded.
* Feeding admin update policy into the update service.

But I haven't been able to talk folks into that yet, so this commit is
piping the relevant data up from ClusterVersion to HostedCluster, and
it can be shipped on from there to whichever actor makes update
decisions.

The lint:ignore comment avoids Staticcheck complaints [1] like [2]:

  cd ./hack/tools; GO111MODULE=on GOFLAGS=-mod=vendor go build -tags=tools -o bin/staticcheck honnef.co/go/tools/cmd/staticcheck
  /go/src/github.com/openshift/hypershift/hack/tools/bin/staticcheck \
  	./control-plane-operator/... \
  	./hypershift-operator/controllers/... \
  	./ignition-server/... \
  	./cmd/... \
  	./support/certs/... \
  	./support/releaseinfo/... \
  	./support/upsert/... \
  	./konnectivity-socks5-proxy/... \
  	./contrib/... \
  	./availability-prober/...
  control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus.go:102:3: hcp.Status.Version is deprecated: Use versionStatus.desired.version instead. +kubebuilder:validation:Optional  (SA1019)
  control-plane-operator/hostedclusterconfigoperator/controllers/hcpstatus/hcpstatus.go:103:3: hcp.Status.ReleaseImage is deprecated: Use versionStatus.desired.image instead. +optional  (SA1019)

[1]: https://staticcheck.io/docs/configuration/#line-based-linter-directives
[2]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_hypershift/1954/pull-ci-openshift-hypershift-main-verify/1612859041380306944#1:build-log.txt%3A280
…lUpdates

I'd recently added these to v1beta1, but it lead to TestFuzzyConversion failures like [1]:

  --- FAIL: TestFuzzyConversion (17.55s)
      --- FAIL: TestFuzzyConversion/for_HostedCluster (13.82s)
          --- FAIL: TestFuzzyConversion/for_HostedCluster/hub-spoke-hub (0.06s)
              fuzz.go:122:
                    &v1beta1.HostedCluster{
                    	TypeMeta:   {},
                    	ObjectMeta: {Name: "實ū", GenerateName: "C", Namespace: "桵ǯƢƋIʦƯ揦Ŭõ", SelfLink: "餹擧ijƊ6ȓŰ蓁<码竆唥鎾It¬O僸戸", ...},
                    	Spec: v1beta1.HostedClusterSpec{
                    		Release:   {Image: "Ĉy篽Õ"},
                    		ClusterID: "ȹR>迨Ǣ軃鄖5!:鏨ĭkīRU鮎",
                  - 		Channel:   "x荓!*Tt缽s览vɦrŸƻț罘`",
                  + 		Channel:   "",
  ...

The fuzzy-conversion injection seems to only be defined for v1alpha1:

  $ git grep 'for HostedCluster' | grep _test.go
  api/v1alpha1/zz_conversion_test.go:     t.Run("for HostedCluster", conversiontest.FuzzTestFunc(conversiontest.FuzzTestFuncInput{

From Cesar:

  Just make the same changes you made to the v1beta1 API to the
  v1alpha1 API. For now, to simplify conversion, we're keeping
  v1alpha1 as a superset of v1beta1 (it's essentially v1beta1 +
  several deprecated fields)

So this commit catches v1alpha1 up with my new v1beta1 properties.

[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_hypershift/1954/pull-ci-openshift-hypershift-main-unit/1603176003130101760
Use v1beta1 as the documented API documentation instead of v1alpha1 which is deprecated
- test(api): Add tracer stmts for HC conversion
- test(api): Fix deprecated API conversion
- test(api): Fix deprecated API conversion v2
- Revert "test(api): Fix deprecated API conversion v2"
- This reverts commit 8a429ba.
- test(api): Fix deprecated API conversion v3
- Revert "test(api): Add tracer stmts for HC conversion"
- This reverts commit 95e9e9a.
- chore: Clean up
- chore: Address review feedback
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 5, 2023

@a-dsouza: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

feat: Backport v1beta1 support to 4.11

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.

@a-dsouza
Copy link
Contributor Author

a-dsouza commented Apr 7, 2023

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 2023

@a-dsouza: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws 1cdcab5 link true /test e2e-aws
ci/prow/e2e-aws-metrics 1cdcab5 link false /test e2e-aws-metrics
ci/prow/unit 1cdcab5 link true /test unit
ci/prow/e2e-conformance 1cdcab5 link false /test e2e-conformance

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@sjenning
Copy link
Contributor

/approve
/area hypershift-operator

@openshift-ci openshift-ci bot added area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Apr 12, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-dsouza, sjenning

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2023
@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 openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 12, 2023
@a-dsouza a-dsouza closed this Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants