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

OAPE-18: Bump openshift/api to vendor machineNamePrefix field #333

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

chiragkyal
Copy link
Member

@chiragkyal chiragkyal commented Dec 2, 2024

Bump openshift/api to vendor machineNamePrefix field and CPMSMachineNamePrefix feature-gate

go get github.com/openshift/api@43830fe27fdad91d7af6ee0da62010507817b3d8
go mod tidy && go mod vendor

Signed-off-by: chiragkyal <[email protected]>
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 2, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 2, 2024

@chiragkyal: This pull request references OAPE-18 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.19.0" version, but no target version was set.

In response to this:

Bump openshift/api to vendor machineNamePrefix field and CPMSMachineNamePrefix feature-gate

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.

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 2, 2024
@openshift-ci openshift-ci bot requested review from damdo and RadekManak December 2, 2024 10:50
@damdo
Copy link
Member

damdo commented Dec 2, 2024

The failure in units look legit @chiragkyal

@chiragkyal
Copy link
Member Author

chiragkyal commented Dec 2, 2024

The failure in units look legit @chiragkyal

Yeah, it's due to the format library for CEL validation. I asked @JoelSpeed about this in Slack. Let me update the ENVTEST_K8S_VERSION

@chiragkyal
Copy link
Member Author

https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-control-plane-machine-set-operator/333/pull-ci-openshift-cluster-control-plane-machine-set-operator-main-unit/1863634503809896448#1:build-log.txt%3A26

unable to fetch hash for requested version: unable to find archive for 1.31.2 (linux,amd64)
exit status 2
KUBEBUILDER_ASSETS="" ./hack/test.sh

Looks like 1.31.2 binaries are not available in upstream yet.

go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
go: downloading sigs.k8s.io/controller-runtime v0.19.3
----
setup-envtest use
Version: 1.31.0

@damdo
Copy link
Member

damdo commented Dec 3, 2024

Yeah, let's use 1.31.0 then.
Thanks

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 3, 2024
@@ -91,7 +91,7 @@ test: generate fmt vet unit ## Run tests.

.PHONY: unit
unit: ## Run only the tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path --bin-dir $(PROJECT_DIR)/bin)" ./hack/test.sh
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path --bin-dir $(PROJECT_DIR)/bin --index https://raw.githubusercontent.com/openshift/api/master/envtest-releases.yaml)" ./hack/test.sh
Copy link
Member Author

Choose a reason for hiding this comment

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

We can change the index reference to this repository in a follow up PR

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it should stay pointing at o/api please, keeping that as a central openshift source of truth

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, thanks for the suggestion.

@chiragkyal
Copy link
Member Author

Looks like I'm hitting an issue related to openshift/api#2074 PR.

              Status: "Failure",
              Message: "admission webhook \"controlplanemachineset.machine.openshift.io\" denied the request: spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.workspace: Internal error: workspace fields should not be set when control plane nodes are in a failure domain: &v1beta1.Workspace{Server:\"test-vcenter\", Datacenter:\"\", Folder:\"\", Datastore:\"\", ResourcePool:\"\", VMGroup:\"\"}",
              Reason: "Forbidden",
              Details: nil,
              Code: 403,
          },
      }
  to match error
      <*matchers.AndMatcher | 0xc000ab4fc0>: {
          Matchers: [
              <*matchers.EqualMatcher | 0xc0009004a0>{
                  Expected: <string>"admission webhook \"controlplanemachineset.machine.openshift.io\" denied the request: spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.workspace: Internal error: workspace fields should not be set when control plane nodes are in a failure domain: &v1beta1.Workspace{Server:\"test-vcenter\", Datacenter:\"\", Folder:\"\", Datastore:\"\", ResourcePool:\"\"}",
              },
          ],

@jcpowermac Could you please help fix the issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Index should only live in the o/api repo, please don't commit this here

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure sounds good, I've removed the file.

update k8s version of envtest

Signed-off-by: chiragkyal <[email protected]>
@chiragkyal
Copy link
Member Author

/jira-refresh

@chiragkyal
Copy link
Member Author

/jira refresh

@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 3, 2024

@chiragkyal: This pull request references OAPE-18 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.19.0" version, but no target version was set.

In response to this:

/jira refresh

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.

@chiragkyal
Copy link
Member Author

/test unit

@jcpowermac
Copy link

Looks like I'm hitting an issue related to openshift/api#2074 PR.

              Status: "Failure",
              Message: "admission webhook \"controlplanemachineset.machine.openshift.io\" denied the request: spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.workspace: Internal error: workspace fields should not be set when control plane nodes are in a failure domain: &v1beta1.Workspace{Server:\"test-vcenter\", Datacenter:\"\", Folder:\"\", Datastore:\"\", ResourcePool:\"\", VMGroup:\"\"}",
              Reason: "Forbidden",
              Details: nil,
              Code: 403,
          },
      }
  to match error
      <*matchers.AndMatcher | 0xc000ab4fc0>: {
          Matchers: [
              <*matchers.EqualMatcher | 0xc0009004a0>{
                  Expected: <string>"admission webhook \"controlplanemachineset.machine.openshift.io\" denied the request: spec.template.machines_v1beta1_machine_openshift_io.spec.providerSpec.value.workspace: Internal error: workspace fields should not be set when control plane nodes are in a failure domain: &v1beta1.Workspace{Server:\"test-vcenter\", Datacenter:\"\", Folder:\"\", Datastore:\"\", ResourcePool:\"\"}",
              },
          ],

@jcpowermac Could you please help fix the issue?

see #325

@chiragkyal
Copy link
Member Author

see #325

Thanks. I hope bec86f9 will make the unit tests happy for now.

@chiragkyal
Copy link
Member Author

/retest-required

Copy link
Contributor

openshift-ci bot commented Dec 3, 2024

@chiragkyal: 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/okd-scos-e2e-aws-ovn bec86f9 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-vsphere-ovn-etcd-scaling bec86f9 link false /test e2e-vsphere-ovn-etcd-scaling

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.

@chiragkyal
Copy link
Member Author

/assign @JoelSpeed @damdo
up for approval

Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

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

/lgtm

/retest-required

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2024
@chiragkyal
Copy link
Member Author

@damdo, could you help in approval as well? Thanks!

Copy link
Member

@damdo damdo left a comment

Choose a reason for hiding this comment

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

/approve

It looks reasonable to me.

Copy link
Contributor

openshift-ci bot commented Dec 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo

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

The pull request process is described here

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 Dec 5, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 3e1ac98 into openshift:main Dec 5, 2024
18 of 20 checks passed
@chiragkyal chiragkyal deleted the bump-api branch December 5, 2024 10:43
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-control-plane-machine-set-operator
This PR has been included in build ose-cluster-control-plane-machine-set-operator-container-v4.19.0-202412051538.p0.g3e1ac98.assembly.stream.el9.
All builds following this will include this PR.

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants