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.

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

Add heuristic func to set plugin workspace base for windows containers correctly #4286

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

6543
Copy link
Member

@6543 6543 commented Nov 2, 2024

regression of #3933

address one concern of #4274

@6543 6543 added bug Something isn't working build_pr_images If set, the CI will build images for this PR and push to Dockerhub labels Nov 2, 2024
@6543
Copy link
Member Author

6543 commented Nov 2, 2024

@cduchenoy you might verify that this works for you! :)

@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Nov 2, 2024

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-4286.surge.sh

Copy link

codecov bot commented Nov 2, 2024

Codecov Report

Attention: Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 27.10%. Comparing base (5139401) to head (125e777).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pipeline/frontend/yaml/compiler/convert.go 85.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4286      +/-   ##
==========================================
+ Coverage   27.06%   27.10%   +0.04%     
==========================================
  Files         379      379              
  Lines       27753    27769      +16     
==========================================
+ Hits         7512     7528      +16     
  Misses      19572    19572              
  Partials      669      669              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cduchenoy
Copy link
Contributor

Hi,

@6543

Thank you so much !

To try it I need to build the Woodpecker server image with this patch using https://woodpecker-ci.org/docs/development/getting-started
is that it?

As a reminder, we are not devs but infrastructure people

@pat-s
Copy link
Contributor

pat-s commented Nov 4, 2024

You can use the tag pull_4286.

@6543
Copy link
Member Author

6543 commented Nov 4, 2024

As a reminder, we are not devs but infrastructure people

@cduchenoy that's what build_pr_image lable is for :)

server image
agent image

@6543
Copy link
Member Author

6543 commented Nov 9, 2024

@cduchenoy did you had time to test it?

@cduchenoy
Copy link
Contributor

Hi,

Sorry but I haven't been able to test yet

It is in production at our customer and testing is complicated

We are in the process of migrating our old gitea/drone stack to the forgejo/woodpecker couple

Once the infrastructure is finalized (well advanced :)) we will be able to test because our infrastructure and no longer that of a customer

We are also preparing the expected PRs and a teams notification plugin

@cduchenoy
Copy link
Contributor

Hi,

I was finally able to test it works!

I had to create the agent for Windows with the patch because I had a grpc version mismatch error

The Windows docker image windows-agent can be found on docker-hub:

docker pull gecoit84/woodpecker-agent:pull_4286

Do you think you can apply the same fix for the steps command?

Indeed, for the pipeline to work you must add the parameter

...
workspace:
  base: C:\tmp

Without that

image

@6543
Copy link
Member Author

6543 commented Nov 15, 2024

I just had an idea witch would not require an heuristic: why not prepend C:\ directly in the docker backend if the backend itselve detects windows as an host ... 🤔 (and no letter was already added...)

@6543 6543 self-assigned this Nov 15, 2024
@cduchenoy
Copy link
Contributor

Yes it's the solution 👍

@6543
Copy link
Member Author

6543 commented Nov 15, 2024

@cduchenoy ok you might have a look at #4381 ... through I have windows container now running and might do actual testing with woodpecker on my end too now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build_pr_images If set, the CI will build images for this PR and push to Dockerhub
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants