Skip to content

Conversation

@mandre
Copy link
Member

@mandre mandre commented Oct 14, 2025

release-4.21 of openshift/cloud-provider-openstack should be based off upstream release-1.34 branch.

There's 1 upstream carry patch since kubernetes#2915 didn't make the 1.34 cut.

Also includes #351 that currently sits in the merge queue.

Lastly, we changed back the go version to 1.24 to match the expected version (for now) in 4.21.

dkokkinos and others added 30 commits June 20, 2025 04:18
…ubernetes#2883)

* handle boolean values correctly when reading from secrets

Previously, the validator failed when attempting to parse boolean fields
like `UseClouds` in `AuthOpts` from secrets. This was due to secrets
being stored as strings, leading to a type mismatch.

Added logic to correctly parse string representations of booleans to
match the expected type in the struct.

* Implement clouds.yaml support and make auth params optional

Enables reading credentials/config from clouds.yaml when UseClouds is
set. Mark Region and AuthURL as optional.
If Openstack quotas for volumes are exceeded, cinder returns with a HTTP
413 response code:
https://github.com/openstack/cinder/blob/c4f61c11ef9b590606a2a276bdf256622516181f/cinder/quota_utils.py#L130-L140

CSI spec defines that on a quota limit error, the driver should return a
ResourceExhausted grpc error code:
https://github.com/container-storage-interface/spec/blob/master/spec.md#createvolume-errors

Signed-off-by: Lukas Hoehl <[email protected]>
Bumps [github.com/go-chi/chi/v5](https://github.com/go-chi/chi) from 5.2.1 to 5.2.2.
- [Release notes](https://github.com/go-chi/chi/releases)
- [Changelog](https://github.com/go-chi/chi/blob/master/CHANGELOG.md)
- [Commits](go-chi/chi@v5.2.1...v5.2.2)

---
updated-dependencies:
- dependency-name: github.com/go-chi/chi/v5
  dependency-version: 5.2.2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Bump k8s.io/kubernetes from 1.33.1 to 1.33.2

Bumps [k8s.io/kubernetes](https://github.com/kubernetes/kubernetes) from 1.33.1 to 1.33.2.
- [Release notes](https://github.com/kubernetes/kubernetes/releases)
- [Commits](kubernetes/kubernetes@v1.33.1...v1.33.2)

---
updated-dependencies:
- dependency-name: k8s.io/kubernetes
  dependency-version: 1.33.2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* Bump the rest k8s package dependencies

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: pýrus <[email protected]>
* tests: Prefer ginkgo timeout

So that we actually get test results.

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Align opts for Cinder, Manila tests

Add a timeout to the Manila job and otherwise move some lines around.

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Suffix image version

While boskos will reap most resources for us, it doesn't reap images
[1]. This has resulted in us using the same image for who knows how long
at this point.

Encode the Ubuntu version to prevent us picking up other version by
mistake.

[1] https://github.com/kubernetes-sigs/boskos/blob/5993cef5a1c719c33c0936d416b7d935058e1204/cmd/janitor/gcp_janitor.py#L46-L88

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Bump DevStack to Epoxy (2025.1)

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Bump amphora image

Use the Ubuntu 24.04 version, rather than the 22.04 version.
This aligns with what we're using for DevStack itself.

Signed-off-by: Stephen Finucane <[email protected]>

* devstack: Remove USE_PYTHON3

It's all Python 3 now, baby.

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Install Ansible from Debian Testing

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Correct broken conditions

Per the Ansible 2.19 porting guide [1].

[1] https://ansible.readthedocs.io/projects/ansible-core/devel/porting_guides/porting_guide_core_2.19.html

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Decrease device detach timeout

Wait less time before retrying.

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
* Bump CSI test timeouts

We have worked around Nova bug #2119114 by lowering the device detach
threshold to 1 second. Unfortunately this still leaves us with a N
seconds of additional runtime, where N is the number of device detaches
incurred by our test suite (since we run tests serially). This has put
us right on the cusp of timeouts, meaning our jobs occasionally pass and
occasionally fail, depending on the node we end up on. Add a bit more
breathing room for the jobs while we wait for the Nova fix.

Note that we do this for both Cinder and Manila to try keep those jobs
consistent where possible.

[1] https://bugs.launchpad.net/nova/+bug/2119114

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Temporarily remove share v1 endpoint

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
* Remove errant unicode

Signed-off-by: Stephen Finucane <[email protected]>

* Bump golangci-lint to latest version

Done by bumping the version in the Makefile and running the
'golangci-lint migrate' command

  go run github.com/golangci/golangci-lint/v2/cmd/[email protected] migrate

This yields a number of news error from staticcheck, listed below, which
we temporarily ignore pending a fix.

  ST1005: error strings should not be capitalized
  ST1023: should omit type .* from declaration; it will be inferred from the right-hand side
  QF1001: could apply De Morgan's law
  QF1003: could use tagged switch on .*
  QF1004: could use strings.ReplaceAll instead
  QF1004: could merge conditional assignment into variable declaration
  QF1008: could remove embedded field ".*" from selector
  QF1011: could omit type .* from declaration; it will be inferred from the right-hand side

We also add some additional configuration to remove the limits on the
number of issues shown by linters, since this will make fixing these
things easier, and remove configuration around concurrency since this
will be auto-configured by default.

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Use lowercase error strings

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Remove redundant type info

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Prefer switch statements over conditional ladders

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Simplify conditionals

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Prefer strings.ReplaceAll to string.Replace with n=-1

Signed-off-by: Stephen Finucane <[email protected]>

* lint: Remove embedded fields from selectors

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
Bumps [k8s.io/kubernetes](https://github.com/kubernetes/kubernetes) from 1.33.3 to 1.33.4.
- [Release notes](https://github.com/kubernetes/kubernetes/releases)
- [Commits](kubernetes/kubernetes@v1.33.3...v1.33.4)

---
updated-dependencies:
- dependency-name: k8s.io/kubernetes
  dependency-version: 1.33.4
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* docs: Add troubleshooting steps for AZ mismatches

Signed-off-by: Stephen Finucane <[email protected]>

* docs: Correct nits with Cinder CSI docs

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
Currently the lbMethod is not set into the svcConfig which
leads to the default lbMethod being used
* cinder-csi-plugin: Don't set segment with empty topology value

This is actually handled by csi-provisioner, but it's arguably
incorrect.

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Don't use global mocks

Calling '.On' with the same expected arguments multiple times will only
result in a single mock: the first one. We should avoid doing this.

We also remove some useless comments that simply duplicate what the
calls already say.

Signed-off-by: Stephen Finucane <[email protected]>

* cinder-csi-plugin: Add tests for ignore-volume-az

Signed-off-by: Stephen Finucane <[email protected]>

* cinder-csi-plugin: Add tests for --with-topology=false

This highlights an issue (IMO) is how we handle parameter generation.

Signed-off-by: Stephen Finucane <[email protected]>

* cinder-csi-plugin: Group topology generation

Group everything together rather than having it spread out as it is
currently. A decoder of the logic, to ensure nothing has changed:

1. Determine what the OpenStack Cinder volume's AZ will be:

  a. If the `availability` parameter is set on the Storage Class, the
     cinder volume will use this as the AZ.
  b. Otherwise, the cinder volume will use: an AZ extracted from one of
     the preferred CSI Topologies; an AZ extracted from one of the
     requisite (available) Topologies; or no AZ.

2. Determine what the Kubernetes CSI Volume's topology will be:

  a. If the `ignore-volume-az` option is set, the CSI Volume will use
     the preferred CSI Volume Topologies, if any, else None. [*]
  b. Otherwise, the CSI Volume will generate a Volume Topology from the
     Cinder Volume's AZ.

[*] This practically means that `ignore-volume-az` is only useful when
    the availability parameter is set on the Storage Class. This in turn
    means the CSI Volume Topology is junk as it has no bearing on the
    "real" topology constraint on the volume.

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
Remove dependency on a couple of fields where the logic was wrong. For
example, these fields do not necessarily depend on a password being set,
as we could be using application credentials.

This prevents manila driver from entering an error state when it finds
unnecessary fields in the clouds.yaml. It now simply ignores them.

Co-authored-by: Stephen Finucane <[email protected]>
…#2998)

This is currently backwards from what you'd expect when the value is
false, with the topology information always being ignored for the Cinder
requests but never ignored for the k8s CSI requests. Modify the former
so that we respect the 'availability' parameter when set - regardless of
the value of --with-topology - and ensure that we never set topology
requirements on the CSI Volume when --with-topology=false.

Signed-off-by: Stephen Finucane <[email protected]>
* Bump ginkgo to v2.26.0

Signed-off-by: Stephen Finucane <[email protected]>

* tests: Replace deprecated ginkgo opts

Resolve the following warning from tests:

  You're using deprecated Ginkgo functionality:
  =============================================
    --ginkgo.noColor is deprecated, use --ginkgo.no-color instead
    Learn more at: https://onsi.github.io/ginkgo/MIGRATING_TO_V2#changed-command-line-flags
    --ginkgo.progress is deprecated .  The functionality provided by
      --progress was confusing and is no longer needed.  Use
      --show-node-events instead to see node entry and exit events
      included in the timeline of failed and verbose specs.  Or you can
      run with -vv to always see all node events.  Lastly,
      --poll-progress-after and the PollProgressAfter decorator now
      provide a better mechanism for debugging specs that tend to get
      stuck.

Signed-off-by: Stephen Finucane <[email protected]>

---------

Signed-off-by: Stephen Finucane <[email protected]>
Simply:

    go get -u github.com/gophercloud/gophercloud/[email protected]
    go mod tidy

Followed by some fixes to a test (my fault: I didn't consider our test
framework part of the public API when I made and backported that change
/o\).

Signed-off-by: Stephen Finucane <[email protected]>
Co-authored-by: Jesse Haka <[email protected]>
@openshift-ci openshift-ci bot requested review from gryf and stephenfin October 14, 2025 13:22
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2025
@mandre
Copy link
Member Author

mandre commented Oct 15, 2025

/retest

1 similar comment
@mandre
Copy link
Member Author

mandre commented Oct 15, 2025

/retest

@mandre
Copy link
Member Author

mandre commented Oct 15, 2025

/test e2e-openstack-csi-cinder

@mandre
Copy link
Member Author

mandre commented Oct 21, 2025

/retest

@mandre
Copy link
Member Author

mandre commented Oct 23, 2025

/retest-required

@mandre
Copy link
Member Author

mandre commented Oct 23, 2025

/test e2e-openstack

@mandre
Copy link
Member Author

mandre commented Oct 24, 2025

Timeout due to resource contention.
/test e2e-openstack

@mandre
Copy link
Member Author

mandre commented Oct 27, 2025

/test e2e-openstack-csi-cinder

@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2025

@mandre: The following test 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/security fa3bfb2 link false /test security

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.

@mandre mandre changed the title Rebase main onto release-1.34 OSASINFRA-3954: Rebase main onto release-1.34 Oct 28, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 28, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 28, 2025

@mandre: This pull request references OSASINFRA-3954 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.21.0" version, but no target version was set.

In response to this:

release-4.21 of openshift/cloud-provider-openstack should be based off upstream release-1.34 branch.

There's 1 upstream carry patch since kubernetes#2915 didn't make the 1.34 cut.

Also includes #351 that currently sits in the merge queue.

Lastly, we changed back the go version to 1.24 to match the expected version (for now) in 4.21.

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.

@mandre
Copy link
Member Author

mandre commented Oct 28, 2025

The tests look good. All the tests ran successfully for for e2e-openstack and we just had a timeout in the latest stages of the job, and we only got 1 failing test in e2e-openstack-csi-cinder due to slow etcd.

/override ci/prow/e2e-openstack
/override ci/prow/e2e-openstack-csi-cinder

@openshift-ci
Copy link

openshift-ci bot commented Oct 28, 2025

@mandre: Overrode contexts on behalf of mandre: ci/prow/e2e-openstack, ci/prow/e2e-openstack-csi-cinder

In response to this:

The tests look good. All the tests ran successfully for for e2e-openstack and we just had a timeout in the latest stages of the job, and we only got 1 failing test in e2e-openstack-csi-cinder due to slow etcd.

/override ci/prow/e2e-openstack
/override ci/prow/e2e-openstack-csi-cinder

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.

@mandre
Copy link
Member Author

mandre commented Oct 28, 2025

/verified by ci

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 28, 2025
@openshift-ci-robot
Copy link

@mandre: This PR has been marked as verified by ci.

In response to this:

/verified by ci

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.

@stephenfin
Copy link

I compared this against upstream and found the following changes:

❯ git diff upstream/release-1.34 --name-status  -- . ':!vendor'
A       .ci-operator.yaml
M       .gitignore
A       .snyk
A       DOWNSTREAM_OWNERS
A       DOWNSTREAM_OWNERS_ALIASES
M       docs/manila-csi-plugin/using-manila-csi-plugin.md
M       examples/manila-csi-plugin/nfs/static-provisioning/preprovisioned-pvc.yaml
M       go.mod
M       hack/bump-charts.sh
M       hack/bump-release.sh
A       images/cinder-csi-plugin/Dockerfile
A       images/cloud-controller-manager/Dockerfile
A       images/manila-csi-plugin/Dockerfile
M       pkg/csi/manila/adapters.go
M       pkg/csi/manila/controllerserver.go
M       pkg/csi/manila/nodeserver.go
M       pkg/csi/manila/options/shareoptions.go
M       pkg/csi/manila/shareadapters/cephfs.go
A       pkg/csi/manila/shareadapters/cephfs_test.go
M       pkg/csi/manila/shareadapters/nfs.go
M       pkg/csi/manila/shareadapters/shareadapter.go
M       tests/e2e/csi/manila/testdriver.go

Nothing unexpected as it's a combination of additional changes to the release-1.34 branch since this was proposed, downstream-specific things (CARRY patches), and the kubernetes#2915 PR.

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2025
@openshift-ci
Copy link

openshift-ci bot commented Oct 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mandre, stephenfin

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-merge-bot openshift-merge-bot bot merged commit f0f7a14 into main Oct 29, 2025
9 of 10 checks passed
@stephenfin stephenfin deleted the sync-main-1.34 branch October 30, 2025 10:08
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. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.