NO-JIRA: use go workspaces #358
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Pipeline controller notification |
openshift-ci-robot
added
the
jira/valid-reference
|
@miyadav: This pull request explicitly references no jira issue. In response to this:
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. |
WalkthroughVendor tree was pruned substantially: numerous Azure SDK for Go azcore, arm, runtime, policy, tracing, streaming, and internal packages were deleted, along with compute/armcompute v5 clients and docs. e2e/go.mod updated indirect deps. Makefile now runs envtest and golangci-lint with -mod=vendor and adds a deps phony target. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (104)
⛔ Files not processed due to max files limit (5)
💤 Files with no reviewable changes (104)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Makefile (3)
7-8: Reduce duplication: export GOWORK/GOFLAGS once.Centralize env flags to avoid drift and make behavior uniform across targets.
Apply:
PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.33.2 -ENVTEST = GOWORK=off go run ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest -GOLANGCI_LINT = GOWORK=off go run ${PROJECT_DIR}/vendor/github.com/golangci/golangci-lint/cmd/golangci-lint +export GOWORK=off +export GOFLAGS=-mod=vendor +ENVTEST = go run ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest +GOLANGCI_LINT = go run ${PROJECT_DIR}/vendor/github.com/golangci/golangci-lint/cmd/golangci-lint @@ - cd manifests-gen && GOWORK=off go build -o ../bin/manifests-gen && cd .. + cd manifests-gen && go build -o ../bin/manifests-gen && cd .. @@ - GOWORK=off go build -o bin/cluster-capi-operator cmd/cluster-capi-operator/main.go + go build -o bin/cluster-capi-operator cmd/cluster-capi-operator/main.go @@ - GOWORK=off go build -o bin/machine-api-migration cmd/machine-api-migration/main.go + go build -o bin/machine-api-migration cmd/machine-api-migration/main.go @@ - GOWORK=off go run cmd/cluster-capi-operator/main.go --images-json=./dev-images.json --leader-elect=true --leader-elect-lease-duration=120s --namespace="openshift-cluster-api" --leader-elect-resource-namespace="openshift-cluster-api" + go run cmd/cluster-capi-operator/main.go --images-json=./dev-images.json --leader-elect=true --leader-elect-lease-duration=120s --namespace="openshift-cluster-api" --leader-elect-resource-namespace="openshift-cluster-api"Also applies to: 32-32, 36-36, 40-40, 60-60
32-32: Minor: use go build -C for readability.Slightly cleaner than subshell cd.
Apply:
-manifests-gen: - # building manifests-gen - cd manifests-gen && go build -o ../bin/manifests-gen && cd .. +manifests-gen: + # building manifests-gen + go build -C manifests-gen -o ../bin/manifests-gen @@ -migration: - # building migration - go build -o bin/machine-api-migration cmd/machine-api-migration/main.go +migration: + # building migration + go build -o bin/machine-api-migration cmd/machine-api-migration/main.goAlso applies to: 40-40
7-8: Confirm vendored tool invocation is stable.Running tools via paths under vendor works today, but some projects relocate tool packages. If that happens, these targets will break. Consider pinning with a toolchain container or a dedicated tools module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
e2e/go.sumis excluded by!**/*.sum
📒 Files selected for processing (102)
Makefile(3 hunks)e2e/go.mod(1 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/CHANGELOG.md(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/LICENSE.txt(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/README.md(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/internal/resource/resource_identifier.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/internal/resource/resource_type.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy/policy.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/resource_identifier.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/resource_type.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/pipeline.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_bearer_token.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_register_rp.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_trace_namespace.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/runtime.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/ci.yml(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud/cloud.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/core.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/errors.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/etag.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/exported.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/pipeline.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/response_error.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/log/log.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/async/async.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/body/body.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/fake/fake.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/loc/loc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/op/op.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/poller.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/util.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared/constants.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared/shared.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/log/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/log/log.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/policy/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/policy/policy.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/errors.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/pager.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/pipeline.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_api_version.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_bearer_token.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_body_download.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_header.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_trace.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_include_response.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_key_credential.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_logging.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_request_id.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_retry.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_sas_credential.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_telemetry.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/poller.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/request.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/response.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_dialer_other.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_dialer_wasm.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_http_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming/progress.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing/constants.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing/tracing.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/LICENSE.txt(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/diag/diag.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/diag/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo/errorinfo.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/exported/exported.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/log/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/log/log.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/poller/util.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/temporal/resource.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/uuid/doc.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/uuid/uuid.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/CHANGELOG.md(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/LICENSE.txt(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/README.md(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/assets.json(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/autorest.md(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/availabilitysets_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/build.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/capacityreservationgroups_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/capacityreservations_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/ci.yml(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/client_factory.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceoperatingsystems_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceroleinstances_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceroles_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudservices_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudservicesupdatedomain_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleries_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleryimages_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleryimageversions_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/dedicatedhostgroups_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/dedicatedhosts_client.go(0 hunks)e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/diskaccesses_client.go(0 hunks)
⛔ Files not processed due to max files limit (9)
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/diskencryptionsets_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/diskrestorepoint_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/disks_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/galleries_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/galleryapplications_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/galleryapplicationversions_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/galleryimages_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/galleryimageversions_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/gallerysharingprofile_client.go
💤 Files with no reviewable changes (100)
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/README.md
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/ci.yml
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/diag/diag.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/CHANGELOG.md
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/LICENSE.txt
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/runtime.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/assets.json
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/README.md
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_sas_credential.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/ci.yml
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_dialer_wasm.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/uuid/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleries_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/exported/exported.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/LICENSE.txt
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_body_download.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/resource_type.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/LICENSE.txt
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/errors.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/CHANGELOG.md
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/autorest.md
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_trace_namespace.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/log/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/poller.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/resource_identifier.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/log/log.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/pager.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_http_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/errors.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/loc/loc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/pipeline.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/response.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared/constants.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_bearer_token.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_include_response.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/policy/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_request_id.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/diag/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_header.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/policy/policy.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing/constants.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/log/log.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/transport_default_dialer_other.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/request.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud/cloud.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy/policy.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/response_error.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/log/log.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo/errorinfo.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/log/doc.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_bearer_token.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_key_credential.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/internal/resource/resource_identifier.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceroles_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/build.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/pipeline.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/runtime/policy_register_rp.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/capacityreservationgroups_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/internal/resource/resource_type.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/poller/util.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceroleinstances_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/op/op.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/client_factory.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_retry.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleryimages_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/poller.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared/shared.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/async/async.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_telemetry.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/core.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/communitygalleryimageversions_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_http_trace.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/body/body.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/dedicatedhostgroups_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudserviceoperatingsystems_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming/progress.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/util.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudservicesupdatedomain_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/temporal/resource.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers/fake/fake.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/capacityreservations_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/dedicatedhosts_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/diskaccesses_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/tracing/tracing.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/exported.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/cloudservices_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/internal/uuid/uuid.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5/availabilitysets_client.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/etag.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported/request.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_api_version.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/pipeline.go
- e2e/vendor/github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime/policy_logging.go
🔇 Additional comments (2)
e2e/go.mod (1)
24-24: vSphere provider bump looks fine; verify e2e coverage.No obvious API breakage expected from v1.13.1, but please ensure e2e vsphere jobs compile/run with this graph and that vendoring is up to date.
Could you confirm:
./hack/vendor.shwas run ande2e/vendorreflects this bump?- e2e tests that exercise vSphere still pass in CI?
Makefile (1)
7-8: Title vs. implementation: GOWORK=off explicitly disables workspaces.If the intent is to “use go workspaces,” this change does the opposite (forces single-module/vendored mode). If that’s intended (for reproducibility), consider clarifying the PR title/description.
|
/test e2e-metal3-capi-techpreview |
|
/test e2e-aws-capi-techpreview |
chrischdi
reviewed
Sep 9, 2025
|
Note: not sure if the script should also include |
|
hey @chrischdi , thanks , I have them here , let me check if they are ignored , you are correct about |
|
Ah sorry for that, looks like the github ui fails for this PR 😂 |
sorry from me too :D , let me make two separate commits. |
was going to comment - if you can separte the vendoring removal (5k+ changes) from the actual code changes, that would be awesome! we also need to see the |
miyadav
force-pushed
the
goworkusagefore2evendoring
branch
from
dcc56eb to
d45b77c
Compare
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Maybe a replace inside go.work for core capi could make |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Makefile (1)
32-32: DRY up repeated GOWORK=off go invocationsDefine a single GO variable and reuse it for build/run targets to reduce duplication and the risk of drift.
+GO := GOWORK=off go -ENVTEST = GOWORK=off go run ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest -GOLANGCI_LINT = GOWORK=off go run ${PROJECT_DIR}/vendor/github.com/golangci/golangci-lint/cmd/golangci-lint +ENVTEST = $(GO) run ${PROJECT_DIR}/vendor/sigs.k8s.io/controller-runtime/tools/setup-envtest +GOLANGCI_LINT = $(GO) run ${PROJECT_DIR}/vendor/github.com/golangci/golangci-lint/cmd/golangci-lint - cd manifests-gen && GOWORK=off go build -o ../bin/manifests-gen && cd .. + cd manifests-gen && $(GO) build -o ../bin/manifests-gen && cd .. - GOWORK=off go build -o bin/cluster-capi-operator cmd/cluster-capi-operator/main.go + $(GO) build -o bin/cluster-capi-operator cmd/cluster-capi-operator/main.go - GOWORK=off go build -o bin/machine-api-migration cmd/machine-api-migration/main.go + $(GO) build -o bin/machine-api-migration cmd/machine-api-migration/main.go - GOWORK=off go run cmd/cluster-capi-operator/main.go --images-json=./dev-images.json --leader-elect=true --leader-elect-lease-duration=120s --namespace="openshift-cluster-api" --leader-elect-resource-namespace="openshift-cluster-api" + $(GO) run cmd/cluster-capi-operator/main.go --images-json=./dev-images.json --leader-elect=true --leader-elect-lease-duration=120s --namespace="openshift-cluster-api" --leader-elect-resource-namespace="openshift-cluster-api"Also applies to: 36-36, 40-40, 60-60
hack/test.sh (1)
36-38: Echo the exact command you executeThe printed command omits the GOWORK=off prefix, which complicates debugging CI logs. Print and run the same command.
-echo ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${TEST_DIRS} -GOWORK=off ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${TEST_DIRS} +CMD="GOWORK=off ${GINKGO} ${GINKGO_ARGS} ${GINKGO_EXTRA_ARGS} ${TEST_DIRS}" +echo ${CMD} +${CMD}hack/vendor.sh (1)
18-22: Make the warning clearly a warning (stderr) and fail early overall
- Send the warning to stderr.
- Consider set -euo pipefail at script top to fail early on other errors while still allowing go work sync to be non-fatal.
-echo "Syncing Go workspace" +echo "Syncing Go workspace" if ! go work sync; then - echo "Warning: go work sync failed due to dependency conflicts. This is expected with the current vsphere provider dependency." - echo "The workspace structure is in place for future use, but individual module vendoring will be used for builds." + >&2 echo "Warning: go work sync failed due to dependency conflicts. This is expected with the current vsphere provider dependency." + >&2 echo "The workspace structure is in place for future use, but individual module vendoring will be used for builds." fi + +# At file top (after shebang): +# set -euo pipefail
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (2)
go.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
Makefile(4 hunks)hack/test.sh(2 hunks)hack/vendor.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
hack/vendor.sh
[warning] 12-12: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (5)
Makefile (2)
7-8: LGTM on tool invocation isolationPrefixing envtest and golangci-lint with GOWORK=off is appropriate to avoid workspace interference.
80-83: Good: deps alias for verify-depsThe deps alias matches prow’s verify-deps expectations. Please confirm the prow job indeed calls make deps.
hack/test.sh (1)
20-20: LGTM: explicit vendor-mode for Ginkgo launcherUsing go run -mod=vendor against the vendored ginkgo keeps tests hermetic.
hack/vendor.sh (2)
15-17: LGTM: root vendoring under GOWORK=offInvoking the local vendor() under GOWORK=off ensures module resolution ignores go.work during vendoring.
24-25: Vendoring prerequisites present: bothhack/toolsandmanifests-gencontaingo.mod, sovendor_in_subdirwill succeed.
|
/test verify-deps |
|
I did try adding workspaces for this repo in the past and it didn't work very well. |
Using go.work for some time already and for me it reasonably works well (using gopls via vscode). Also k/k uses go.work and I think it would have been a nightmare if code editors would fail nowadays with that. |
|
@miyadav : with the following We should also not need |
I would hope so yeah! Happy to add them if they work well :) |
|
I also tried go work in this pr, seems works well. |
miyadav
force-pushed
the
goworkusagefore2evendoring
branch
from
823b1cb to
7cfc2db
Compare
great @sunzhaohua2 , lets see if this one works , will rebase my open PRs as well .. |
|
/test verify-deps |
|
/test e2e-openstack-ovn-techpreview |
damdo
reviewed
Sep 17, 2025
Comment on lines
+80
to
+83
| # Alias for vendor (used by verify-deps) | ||
| .PHONY: deps | ||
| deps: vendor | ||
|
|
There was a problem hiding this comment.
I can't find where this is used
I checked here on the verify-deps job config for ccapio, but that doesn't seem to be using this make target
There was a problem hiding this comment.
for make target , it seems harmless just that it will call again ./hack/vendor.sh , just more intuitive..?
There was a problem hiding this comment.
@damdo the verify-deps job is a special one from ART, that ensures konflux / brew can build the repo (e.g with hermetic builds). We shouldnt touch it.
There was a problem hiding this comment.
@theobarberbany I was not implying we should change the verify-deps one :)
I was just following the above comment and not undestanding what it meant
# Alias for vendor (used by verify-deps)
If we know that this alias is not used by anything and is just an alias I suggest we drop it
|
@miyadav I am still seeing the |
Don't think we should have them, after the ./hack/vendor.sh script is adjusted. I am testing it in local by removing it and checking if somewhere we are running |
I'm not sure I'm following this 😓 I think the aim is to remove |
yes @theobarberbany , I just wasn't sure , how it is still present , removed it now and pushed again. I thought may be somewhere we ran go mod vendor in e2e in scripts , which brought them back. |
|
@miyadav: This pull request explicitly references no jira issue. In response to this:
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. |
|
/test remaining-required Scheduling tests matching the |
|
/test e2e-aws-ovn-serial-1of2 |
|
/approve Thanks a lot for setting this up @miyadav ! |
|
/verified bypass Units/CI passing should be ok, this is a tooling change |
openshift-ci-robot
added
the
verified
|
@damdo: The In response to this:
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
bot
added
the
approved
|
/lgtm |
Ahh I see - makes sense :) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo, theobarberbany 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 |
|
/override ci/prow/e2e-openstack-ovn-techpreview I've seen this passing, but it times out a lot, let's override to get this merged |
|
/skip |
|
@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-openstack-ovn-techpreview In response to this:
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. |
|
@miyadav: The following test failed, say
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@theobarberbany @damdo PTAL when time permits -
cc @sunzhaohua2 @huali9
Will let the regression runs and review results .
Summary by CodeRabbit