Skip to content

Conversation

@baum
Copy link

@baum baum commented Jul 22, 2025

Add build-time ceph-nvmeof protobuf generation infrastructure

Summary

This PR implements protobuf stubs generation infrastructure that uses committed gateway.proto files and generates Go stubs when needed.

Changes

  • build.env: Added protobuf tool versions and configurable proto source (repo, branch, SHA)
  • scripts/generate-proto.sh: Protobuf stub generation, logging, and error handling
  • Makefile: Added generate-proto, clean-proto, force-generate-proto targets and updated clean target
  • scripts/Dockerfile.devel: Updated to install protobuf tooling using versions from build.env
  • deploy/cephcsi/image/Dockerfile: Added protobuf dependencies for CI builds
  • .github/workflows/check-proto-changes.yml: New CI workflow to validate proto file matches build.env specification

Configuration

The system uses build.env for centralized configuration:

# Proto source configuration
PROTO_SOURCE_REPO=https://github.com/ceph/ceph-nvmeof
PROTO_SOURCE_BRANCH=devel
PROTO_SOURCE_SHA=latest  # or specific SHA

# Protobuf tool versions
PROTOC_VERSION=3.14.0
PROTOC_GEN_GO_VERSION=v1.31.0
PROTOC_GEN_GO_GRPC_VERSION=v1.3.0

File Structure

internal/nvme/gateway/proto/
├── gateway.proto          # Committed source file
├── gateway.pb.go          # Generated Go stubs
└── gateway_grpc.pb.go     # Generated gRPC stubs

Usage

Containerized Generation (Recommended)

# Generate protobuf stubs in a container (no local setup required)
make containerized-generate-proto

Local Development

# Install dependencies first (use versions from build.env)
sudo apt-get install protobuf-compiler  # Ubuntu/Debian
# For CentOS: sudo yum install protobuf-compiler
go install google.golang.org/protobuf/cmd/[email protected]
go install google.golang.org/grpc/cmd/[email protected]

# Generate stubs
make generate-proto

Available Makefile Targets

# Generate protobuf stubs from committed files
make generate-proto

# Clean generated files only
make clean-proto

# Force regeneration (clean first, then generate)
make force-generate-proto

# Containerized generation
make containerized-generate-proto

# Full build with proto generation
make cephcsi

For Developers (IDE Development)

If you've just cloned the codebase and want to generate protobuf stubs for your IDE:

Quick Start (Recommended)

# Generate protobuf stubs in a container (no local setup required)
make containerized-generate-proto

This will:

  • Build the development container with all dependencies
  • Use committed gateway.proto from the repository
  • Generate Go stubs in internal/nvme/gateway/proto/
  • Make your IDE happy with proper gRPC type recognition

See also

Describe what this PR does

Provide some context for the reviewer

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@baum baum force-pushed the ceph_nvmeof_proto branch from 9f796d9 to be59b2c Compare July 22, 2025 10:03
@baum baum force-pushed the ceph_nvmeof_proto branch 6 times, most recently from d19f055 to 11aa3aa Compare July 22, 2025 10:39
@nixpanic nixpanic added component/build Issues and PRs related to compiling Ceph-CSI component/nvme-of Issues and PRs related to NVMe-oF. labels Jul 23, 2025
@nixpanic
Copy link
Member

nixpanic commented Jul 23, 2025

Hi @baum , thanks for the PR!

This looks pretty complete, and we can take this approach as an alternative for ceph/ceph-nvmeof#1360.

There are a few minor changes that I would like to see:

  1. no need to always generate the files, only on-demand
  2. internal/nvmeof/gateway/proto/gateway.proto can be the location, with generated protobuf/gRPC in the same directory
  3. the generated files can be checked in
  4. versions of the Go tools should come from build.env so that release branches stick to a known good version
  5. a CI job to check changes to gateway.proto so we're forced to keep the devel branch up-to-date

Let me know if you want to take care of them, or if you prefer me to handle it (I'll take care of the CI job in any case).

@baum baum force-pushed the ceph_nvmeof_proto branch 13 times, most recently from 714e982 to 51ce62e Compare July 23, 2025 18:23
@baum baum force-pushed the ceph_nvmeof_proto branch from 51ce62e to 3f710c5 Compare July 23, 2025 18:31
@baum baum closed this Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/build Issues and PRs related to compiling Ceph-CSI component/nvme-of Issues and PRs related to NVMe-oF.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants