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 PVC for agent #163

Merged
merged 23 commits into from
Mar 4, 2024
Merged

Add PVC for agent #163

merged 23 commits into from
Mar 4, 2024

Conversation

pat-s
Copy link
Contributor

@pat-s pat-s commented Feb 15, 2024

To persist /etc/woodpecker/agent.conf and solve woodpecker-ci/woodpecker#3023

To clean old agents, the following commands can be executed on the database (Postgres example)

delete from agents where capacity=-1;
delete from agents where coalesce(name, '') = '';

This will clean a lot stale agents but not all.

Another way is to clean all agents for which the last contact is older than 1 day (thanks @zc-devs), one can do

delete from agents where last_contact < now() - interval '1 day';

Alternatively, one can remove all agents, then recreate the pod and the attached PV. This will reinitialize a fresh agent with a new ID.

I've tested this PR and recreated the agents (two replicas) many times. No new agents were created and /etc/woodpecker/agent.conf always showed the persistet agent ID.

@pat-s pat-s added the feature 🚀️ Add new feature label Feb 15, 2024
@pat-s pat-s marked this pull request as ready for review February 15, 2024 19:27
@pat-s
Copy link
Contributor Author

pat-s commented Feb 15, 2024

@zc-devs @ymettier This will fix the agent registration issue. Any comments/suggestions?

Copy link

@zc-devs zc-devs left a comment

Choose a reason for hiding this comment

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

To clean old agents, the following commands can be executed on the database

Maybe it worth to mention query like

delete from agents where last_contact < now() - interval '1 day';

^ not tested.

@pat-s pat-s requested a review from anbraten February 18, 2024 12:43
@pat-s
Copy link
Contributor Author

pat-s commented Feb 27, 2024

ping @anbraten

charts/woodpecker/charts/agent/templates/statefulset.yaml Outdated Show resolved Hide resolved
charts/woodpecker/charts/agent/values.yaml Outdated Show resolved Hide resolved
charts/woodpecker/charts/agent/values.yaml Outdated Show resolved Hide resolved
charts/woodpecker/charts/agent/values.yaml Outdated Show resolved Hide resolved
@pat-s
Copy link
Contributor Author

pat-s commented Mar 4, 2024

@xoxys Added yamllint, prettierc, addressed all warnings and your comments.

Copy link
Member

@xoxys xoxys left a comment

Choose a reason for hiding this comment

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

LGTM

@pat-s pat-s merged commit 2113563 into main Mar 4, 2024
3 checks passed
@pat-s pat-s deleted the agent-config-persistence branch March 4, 2024 19:37
@woodpecker-bot woodpecker-bot mentioned this pull request Mar 2, 2024
1 task
# -- Defines an existing claim to use
existingClaim:
# -- Defines the size of the persistent volume
size: 1Mi
Copy link

Choose a reason for hiding this comment

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

is this supposed to be 1Gi?

Copy link
Member

Choose a reason for hiding this comment

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

Why? The volume just holds the agent configuration (text file). 1Mi should be more than enough as default.

Copy link

Choose a reason for hiding this comment

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

it's 1Gi in the agent subchart values (https://github.com/woodpecker-ci/helm/pull/163/files#diff-afab925868b45a9b5ea72b370133adf7a55e74652cfd7695bbeca6b7e2c30658R65) and i think 1Mi is below the minimum of some csi provisioners

Copy link
Member

Choose a reason for hiding this comment

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

Ok yes, missed that. Good point in that case we should fix it.

@pat-s pat-s mentioned this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🚀️ Add new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants