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.

Sign up for GitHub

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

Jump to bottom

CARRY: Fix vendoring #265

Merged
merged 5 commits into from
Oct 5, 2023
Merged

CARRY: Fix vendoring #265

merged 5 commits into from
Oct 5, 2023

Conversation

mdbooth
Copy link

@mdbooth mdbooth commented Oct 4, 2023 •
edited

In separate commits:

@mdbooth
Copy link
Author

mdbooth commented Oct 4, 2023

We need kubernetes-sigs#1707 to fix test.

@mdbooth
Copy link
Author

mdbooth commented Oct 4, 2023

openshift/release#43982 should prevent us pushing without out-of-date vendoring again.

@mdbooth mdbooth mentioned this pull request Oct 4, 2023
Closed
3 tasks
@mdbooth mdbooth changed the title Fix vendoring CARRY: Fix vendoring Oct 4, 2023
@EmilienM
Copy link
Member

EmilienM commented Oct 4, 2023

/lgtm
/approve

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

openshift-ci bot commented Oct 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EmilienM

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 Oct 4, 2023
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 62eecb8 and 2 for PR HEAD 6329538 in total

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2023
@mdbooth mdbooth mentioned this pull request Oct 4, 2023
Merged
@mdbooth
Copy link
Author

mdbooth commented Oct 4, 2023

Needs openshift/release#43988 to merge first.

@mandre
Copy link
Member

mandre commented Oct 5, 2023

/retest

@mandre
Copy link
Member

mandre commented Oct 5, 2023

Now failing with:

msg: "failed to start the controlplane. retried 5 times: unable to run command \"kube-apiserver\" to check for flag \"insecure-port\": exec: \"kube-apiserver\": executable file not found in $PATH",

@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

It's still failing due to envtest-setup:

make[1]: Leaving directory '/go/src/sigs.k8s.io/cluster-api-provider-openstack/hack/tools'
unable to make sure store is initialized: unable to make sure base binaries dir exists: mkdir /.local: permission denied
KUBEBUILDER_ASSETS="" go test -v ./...

We can see that XDG_DATA_HOME is set correctly, though:

XDG_DATA_HOME=/tmp/data

@mandre
Copy link
Member

mandre commented Oct 5, 2023

We can see that XDG_DATA_HOME is set correctly, though:

XDG_DATA_HOME=/tmp/data

Try XDG_DATA_HOME=/tmp/.local/share XDG_CACHE_HOME=/tmp/.cache. This is what I had to do in openshift/release#39007.

@mandre
Copy link
Member

mandre commented Oct 5, 2023

We can see that XDG_DATA_HOME is set correctly, though:

XDG_DATA_HOME=/tmp/data

Try XDG_DATA_HOME=/tmp/.local/share XDG_CACHE_HOME=/tmp/.cache. This is what I had to do in openshift/release#39007.

Hmm, re-reading your error message, I'm not sure it's gonna help.

@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

I've added a DO NOT MERGE patch which hardcodes a value which needs to go into prow once we've worked out what it needs to be.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2023
@mdbooth
UPSTREAM 1707: Build setup-envtest in hack/tools
0709d3e 
This makes the build of setup-envtest consistent with the other tools
built in hack/tools. By moving the dependency to go.mod, it also allows
the build to work correctly when using enforced vendoring.

(cherry picked from commit 757c7e6)

Conflicts:
	hack/tools/go.sum
@mdbooth
CARRY: go mod vendor
cbcff39
@mdbooth
UPSTREAM 1710: Allow custom KUBEBUILDER_ASSETS_DIR in tests
4995309 
The primary purpose of this change is to enable running the envtests in
environments where neither $XDG_DATA_HOME nor $HOME are set.

Additionally, by executing setup-envtest explicitly in the `test` rule
rather than in a global variable definition, we:
* Only run it when it's required
* Log its execution and output

(cherry picked from commit c076ff9)
@mdbooth
UPSTREAM 1709: Ignore vendor directories in boilerplate check
8bb50a1 
(cherry picked from commit 1dd7990)
@mdbooth mdbooth mentioned this pull request Oct 5, 2023
Merged
@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

/retest
/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2023
@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

/retest required

@openshift-ci
Copy link

openshift-ci bot commented Oct 5, 2023

@mdbooth: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test test

Use /test all to run all jobs.

In response to this:

/retest required

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/test-infra repository.

@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

/retest-required

@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

/test all

mandre
mandre reviewed Oct 5, 2023
View reviewed changes
Copy link
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

Pending tests, but tide prevents this PR to merge until tests pass anyway. Feel free to remove the hold when you're happy with CI.

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Oct 5, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 5, 2023

@mdbooth: all tests passed!

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/test-infra repository. I understand the commands that are listed here.

@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2023
@mdbooth
Copy link
Author

mdbooth commented Oct 5, 2023

@mandre For your enjoyment: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-api-provider-openstack/265/pull-ci-openshift-cluster-api-provider-openstack-main-test/1709939542892482560/artifacts/test/build-log.txt 🍾

@openshift-ci openshift-ci bot merged commit 44086dc into openshift:main Oct 5, 2023
3 checks passed
@mdbooth mdbooth deleted the vendoring branch October 5, 2023 15:03
@mdbooth mdbooth mentioned this pull request Oct 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mandre mandre mandre left review comments

@EmilienM EmilienM Awaiting requested review from EmilienM

@pierreprinetti pierreprinetti Awaiting requested review from pierreprinetti

Assignees

@mandre mandre

@EmilienM EmilienM

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mdbooth @EmilienM @openshift-ci-robot @mandre