openstack-k8s-operators dependency bump branch: main

[github-actions]

Automated changes by create-pull-request GitHub action

Jira: OSPRH-12935

[openshift-ci]

Hi @github-actions[bot]. Thanks for your PR.

I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[stuggi]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: github-actions[bot], stuggi

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:

• OWNERS [stuggi]

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

[stuggi]

/retest

[abays]

@github-actions[bot]: 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/openstack-operator-build-deploy-kuttl 98b488b link true /test openstack-operator-build-deploy-kuttl
ci/prow/openstack-operator-build-deploy-kuttl-4-18 98b488b link true /test openstack-operator-build-deploy-kuttl-4-18

Full PR test history. Your PR dashboard.

{  [failed to wait for the created cluster claim to become ready: timed out waiting for the condition, failed to delete cluster claim b5e5a518-f12d-4817-b91f-b6c9a6fa2399 in namespace openstack-k8s-operators-cluster-pool: Delete "https://api.hosted-mgmt.ci.devcluster.openshift.com:6443/apis/hive.openshift.io/v1/namespaces/openstack-k8s-operators-cluster-pool/clusterclaims/b5e5a518-f12d-4817-b91f-b6c9a6fa2399": dial tcp 34.196.196.82:6443: i/o timeout]}

Prow is still broken and has been for hours at this point.

[stuggi]

/retest

[stuggi]

/retest

[openshift-ci]

New changes are detected. LGTM label has been removed.

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/98f9f1f231584363a42e6df8b6f80560

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 17m 24s
❌ podified-multinode-edpm-deployment-crc FAILURE in 34m 23s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 24m 22s
❌ adoption-standalone-to-crc-ceph-provider FAILURE in 2h 02m 08s
❌ openstack-operator-tempest-multinode FAILURE in 37m 17s
✔️ openstack-operator-docs-preview SUCCESS in 2m 30s

[rabi]

Looks like removing omitempty as expected by operator-lint in openstack-k8s-operators/openstack-baremetal-operator#324 is causing issues, when bumping openstack-baremetal-operator.

@gibizer can you help check this? When using operator-lint 0.3.0, it cribbed about https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openstack-k8s-operators_openstack-baremetal-operator/326/pull-ci-openstack-k8s-operators-openstack-baremetal-operator-main-precommit-check/1971089793609633792 and I removed omitempty from those fields to comply. However, it causes issues when the type is used in openstack-operator https://github.com/openstack-k8s-operators/openstack-operator/blob/main/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go#L40 and a CR leaves out baremetalSetTemplate field.

openstackdataplanedeployment.dataplane.openstack.org/edpm-deployment created
The OpenStackDataPlaneNodeSet "openstack-edpm-ipam" is invalid: spec.baremetalSetTemplate.automatedCleaningMode: Unsupported value: "": supported values: "metadata", "disabled"

[rabi]

This is what we've in BMO https://github.com/metal3-io/baremetal-operator/blob/main/apis/metal3.io/v1alpha1/baremetalhost_types.go#L481-L483 which has all the three (default, optional, omitempty)

[rabi]

This is what we've in BMO https://github.com/metal3-io/baremetal-operator/blob/main/apis/metal3.io/v1alpha1/baremetalhost_types.go#L481-L483 which has all the three (default, optional, omitempty)

I've reverted back to what was there before with openstack-k8s-operators/openstack-baremetal-operator#326

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/034a61fc75d248288c10800f14df1787

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 23m 24s
❌ podified-multinode-edpm-deployment-crc FAILURE in 38m 16s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 37m 29s
❌ adoption-standalone-to-crc-ceph-provider FAILURE in 2h 05m 32s
❌ openstack-operator-tempest-multinode FAILURE in 39m 07s
✔️ openstack-operator-docs-preview SUCCESS in 2m 33s

[gibizer]

Looks like removing omitempty as expected by operator-lint in openstack-k8s-operators/openstack-baremetal-operator#324 is causing issues, when bumping openstack-baremetal-operator.

@gibizer can you help check this? When using operator-lint 0.3.0, it cribbed about https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openstack-k8s-operators_openstack-baremetal-operator/326/pull-ci-openstack-k8s-operators-openstack-baremetal-operator-main-precommit-check/1971089793609633792 and I removed omitempty from those fields to comply. However, it causes issues when the type is used in openstack-operator https://github.com/openstack-k8s-operators/openstack-operator/blob/main/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go#L40 and a CR leaves out baremetalSetTemplate field.

openstackdataplanedeployment.dataplane.openstack.org/edpm-deployment created
The OpenStackDataPlaneNodeSet "openstack-edpm-ipam" is invalid: spec.baremetalSetTemplate.automatedCleaningMode: Unsupported value: "": supported values: "metadata", "disabled"

The error message says that
the field cannot be set to "" as that is not an allowed value for the field

// AutomatedCleaningMode is the interface to enable/disable automated cleaning
// +kubebuilder:validation:Enum=metadata;disabled
type AutomatedCleaningMode string

indeed the field is defined as a string with an enum validation with two possible value. So it cannot be set to "".

If I have to guess what is missing is a proper initialization / defaulting of the usage of OpenStackBaremetalSetTemplateSpec that embeds the field. If a field with type OpenStackBaremetalSetTemplateSpec is not initialized then go will initialize it with the type's null value, which for a (non pointer) struct will be a recursive initialization of the struct's field each with the null value. A string field's null value in golang in the "". So from go perspective it needs to be set to "" but from validation perspective that is not a valid value.

I suggest to look into how we define / default / initialize the field that has type OpenStackBaremetalSetTemplateSpec.

In general golang / k8s JSON SER/DES is hard.

Also I'm OK to ignore the lint and keep the omitempty as I have not time to dig deeper into the issue and offer a full solution.

[rabi]

If I have to guess what is missing is a proper initialization / defaulting of the usage of OpenStackBaremetalSetTemplateSpec that embeds the field.

Thanks! It's defaulted[1] in the OpenStackBaremetalSetTemplateSpec struct itself like what's in BMO[2].

[1] https://github.com/openstack-k8s-operators/openstack-baremetal-operator/blob/main/api/v1beta1/openstackbaremetalset_types.go#L77
[2] https://github.com/metal3-io/baremetal-operator/blob/main/apis/metal3.io/v1alpha1/baremetalhost_types.go#L481-L483

[gibizer]

If I have to guess what is missing is a proper initialization / defaulting of the usage of OpenStackBaremetalSetTemplateSpec that embeds the field.

Thanks! It's defaulted[1] in the OpenStackBaremetalSetTemplateSpec struct itself like what's in BMO[2].

[1] https://github.com/openstack-k8s-operators/openstack-baremetal-operator/blob/main/api/v1beta1/openstackbaremetalset_types.go#L77 [2] https://github.com/metal3-io/baremetal-operator/blob/main/apis/metal3.io/v1alpha1/baremetalhost_types.go#L481-L483

I meant looking at a struct with a field with a type of OpenStackBaremetalSetTemplateSpec. We saw before that defaulting fields that has a type that is a struct is tricky. E.g. I remember I had to do these type of defaulting on the high level field def to avoid similar problems in the past https://github.com/openstack-k8s-operators/nova-operator/blob/a6113c8dcb738f50941c4d81f96afb199890207a/api/v1beta1/nova_types.go#L53-L58

[rabi]

I meant looking at a struct with a field with a type of OpenStackBaremetalSetTemplateSpec. We saw before that defaulting fields that has a type that is a struct is tricky. E.g. I remember I had to do these type of defaulting on the high level field def to avoid similar problems in the past https://github.com/openstack-k8s-operators/nova-operator/blob/a6113c8dcb738f50941c4d81f96afb199890207a/api/v1beta1/nova_types.go#L53-L58

Yeah I tried to use {} default for OpenStackBaremetalSetTemplateSpec but that did not work as there are required fields in the struct[1] which can't be defaulted.

[1] https://github.com/openstack-k8s-operators/openstack-baremetal-operator/blob/main/api/v1beta1/openstackbaremetalset_types.go#L92

[rabi]

dataplane functional tests pass for me locally, let me re-run that.

[rabi]

/test functional

[stuggi]

something is wrong that those get removed

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/0360402840e8489f8f2b6cfe0187a033

❌ openstack-k8s-operators-content-provider FAILURE in 15m 51s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ adoption-standalone-to-crc-ceph-provider SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

[gibizer]

I meant looking at a struct with a field with a type of OpenStackBaremetalSetTemplateSpec. We saw before that defaulting fields that has a type that is a struct is tricky. E.g. I remember I had to do these type of defaulting on the high level field def to avoid similar problems in the past https://github.com/openstack-k8s-operators/nova-operator/blob/a6113c8dcb738f50941c4d81f96afb199890207a/api/v1beta1/nova_types.go#L53-L58

Yeah I tried to use {} default for OpenStackBaremetalSetTemplateSpec but that did not work as there are required fields in the struct[1] which can't be defaulted.

[1] https://github.com/openstack-k8s-operators/openstack-baremetal-operator/blob/main/api/v1beta1/openstackbaremetalset_types.go#L92

Hm if that struct has mandatory fields, then the usage of this struct here

openstack-operator/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go

Lines 38 to 40 in 8013333

	// +kubebuilder:validation:Optional
	// BaremetalSetTemplate Template for BaremetalSet for the NodeSet
	BaremetalSetTemplate baremetalv1.OpenStackBaremetalSetTemplateSpec `json:"baremetalSetTemplate,omitempty"`

cannot be optional. I think that is the conflict causing this issue. I.e. There is no way to say that the field is optional if some of its sub fields are mandatory. My immediate reaction is to make BaremetalSetTemplate a pointer not a value. That way it can be optional with a value of nil. But I did not have the full view of what that change can cause elsewhere so I'm not suggesting to do that blindly.

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/969382d8d7fb40ddb04120e63b7fc3b0

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 27m 04s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 21m 28s
❌ cifmw-crc-podified-edpm-baremetal RETRY_LIMIT in 14m 58s
✔️ adoption-standalone-to-crc-ceph-provider SUCCESS in 3h 07m 49s
✔️ openstack-operator-tempest-multinode SUCCESS in 1h 41m 51s
✔️ openstack-operator-docs-preview SUCCESS in 2m 52s

[stuggi]

recheck

[karelyatin]

/test openstack-operator-build-deploy-kuttl-4-18
unrelated cert mismatch, other kuttl job passed

[rabi]

then the usage of this struct here cannot be optional.

Thanks. It's weird, but that's how go works it seems i.e. "initialize all fields with 0 and empty string unless it's a pointer". I think we can convert it to a pointer.

[karelyatin]

/test openstack-operator-build-deploy-kuttl
cluster claim

[stuggi]

/test openstack-operator-build-deploy-kuttl

[stuggi]

/retest

[karelyatin]

/test openstack-operator-build-deploy-kuttl-4-18

[stuggi]

/retest

[stuggi]

/retest