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

Refresh apt cache before installing packages #179

Merged
merged 20 commits into from
Oct 17, 2023
Merged

Refresh apt cache before installing packages #179

merged 20 commits into from
Oct 17, 2023

Conversation

widhalmt
Copy link
Member

@widhalmt widhalmt commented Jun 15, 2023 •
edited
Loading

Run an apt refresh at the start of each role. We can use grace time so we don't neet to refresh it every time and safe some time.

fixes #167

@widhalmt
Refresh apt cache if package is not yet installed
8e3931d 
I know, checking for the existence of the configuration directory is not
the best way. But it's way faster than quering the package manager every
time.

fixes #167
@widhalmt widhalmt added the feature New feature or request label Jun 15, 2023
@widhalmt widhalmt added this to the 1.1.0 milestone Jun 15, 2023
@widhalmt widhalmt self-assigned this Jun 15, 2023
@widhalmt widhalmt enabled auto-merge June 15, 2023 15:17
@widhalmt
Copy link
Member Author

Looks like the current version of Molecule lacks the docker driver.

@widhalmt
Copy link
Member Author

It seems like we need to merge #182 before continuing with this.

@widhalmt
Copy link
Member Author

widhalmt commented Aug 9, 2023

Looks like tests are failing due to long running being "too long running"

@lcndsmr
Copy link
Member

lcndsmr commented Aug 10, 2023

Didn't we fix the tests? Can we merge main?

@widhalmt
Copy link
Member Author

I thought the tests were fixed. :-( As much as I see, tests are still timing out. I don't want to remove the thorough tests we built so far. So I made tests "non mandatory" and check them manually. While this is ultra cumbersome I guess it's the best solution we have so far. Even more so as @martialblog said, we'll have our own GitHub runners soon. And then we'll definitely want our checks back.

So please see the checks "more like guidelines, anyway" and approve if the code makes sense. Even when some checks time out.

@widhalmt
Copy link
Member Author

I found this line https://github.com/NETWAYS/ansible-collection-elasticstack/pull/240/files#diff-85634248b446d8a9a40ae7b3fca510c581832cbc85200a147c0fd9faea85d5d2R30 in a role by @geerlingguy . Sometimes the best solution can be so easy. You just have to think of it. 🤦🏽

lcndsmr
lcndsmr requested changes Aug 23, 2023
View reviewed changes
Copy link
Member

@lcndsmr lcndsmr left a comment

Choose a reason for hiding this comment

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

I am a linter

roles/beats/tasks/main.yml Outdated Show resolved Hide resolved
roles/elasticsearch/tasks/main.yml Outdated Show resolved Hide resolved
roles/kibana/tasks/main.yml Outdated Show resolved Hide resolved
roles/logstash/tasks/main.yml Outdated Show resolved Hide resolved
@widhalmt
Copy link
Member Author

Thanks so much @dnssmr I guess, I'll take a while before I adapted my old brain to this new style of coding. 🤦🏽

@widhalmt
Copy link
Member Author

I hope, I caught all of them, now.

@widhalmt widhalmt marked this pull request as draft August 23, 2023 08:52
auto-merge was automatically disabled August 23, 2023 08:52

Pull request was converted to draft

@widhalmt
Copy link
Member Author

I converted this to da draft because the tests found some error that's not just due to problems in GitHub runners. I'll investigate locally and then come back. On the other hand... that's out of the scope of this PR, so I'll disable Draft status. 🤦🏽

@widhalmt widhalmt marked this pull request as ready for review August 23, 2023 08:54
@widhalmt widhalmt enabled auto-merge August 23, 2023 08:54
@widhalmt widhalmt mentioned this pull request Aug 23, 2023
Closed
@widhalmt widhalmt mentioned this pull request Aug 23, 2023
Merged
@widhalmt
Disable change for apt refresh
7223d12 
Apt refresh broke idempotence tests in another branch
@widhalmt
Disable changed for apt update
abc722b 
It breaks idempotence tests
@widhalmt widhalmt changed the title Refresh apt cache if package is not yet installed Refresh apt cache before installing packages Aug 23, 2023
@widhalmt widhalmt added this pull request to the merge queue Oct 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 13, 2023
@widhalmt
Copy link
Member Author

We need #279 merged for this to work and be merged.

@widhalmt widhalmt enabled auto-merge October 17, 2023 07:26
@widhalmt widhalmt added this pull request to the merge queue Oct 17, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2023
@lcndsmr lcndsmr added this pull request to the merge queue Oct 17, 2023
Merged via the queue into main with commit d429b15 Oct 17, 2023
80 checks passed
@lcndsmr lcndsmr deleted the feature/aptcache-167 branch October 17, 2023 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcndsmr lcndsmr lcndsmr approved these changes

Assignees

@widhalmt widhalmt

Labels
feature New feature or request
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

Update Debian apt cache when packages are not installed
2 participants
@widhalmt @lcndsmr