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

countme: Force disable Count Me logic in DNF #2746

Merged
merged 1 commit into from
Apr 16, 2021

Conversation

travier
Copy link
Member

@travier travier commented Apr 13, 2021

Make sure that we do not use the internal Count Me logic in DNF in
rpm-ostree as we have our own external implementation that is aware of
the different behavior regarding repo handling.

See also the discussions in:

@travier
Copy link
Member Author

travier commented Apr 13, 2021

Needs proper testing but as I'm wondering where I should factorize setting this option, I opened a PR to discuss that.

@cgwalters
Copy link
Member

I think we can likely just change this in the core in rpmostree_context_setup() right?

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Cool! I think with this we can drop the sed instructions in the countme docs now too, right?

@@ -842,6 +842,9 @@ rpmostree_context_setup (RpmOstreeContext *self,
DNF_TRANSACTION_FLAG_NODOCS);
}

/* Force disable internal libdnf Count Me logic */
dnf_conf_add_setopt("*.countme", DNF_CONF_COMMANDLINE, "false", NULL);
Copy link
Member

Choose a reason for hiding this comment

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

Minor/optional: should this be in rpmostree_context_new_system instead? This is where we disable other global-ish librpm/libdnf bits we know we never want upfront.

Make sure that we do not use the internal Count Me logic in DNF in
rpm-ostree as we have our own external implementation that is aware of
the different behavior regarding repo handling.

See also the discussions in:
  - rpm-software-management/libdnf#1174
  - rpm-software-management/libdnf#1068
  - coreos#2671

Also remove the corresponding note in the docs which not needed anymore.
@travier
Copy link
Member Author

travier commented Apr 14, 2021

Updated. This still needs testing and I'm wondering how I'll to do that 🤔

@jlebon
Copy link
Member

jlebon commented Apr 14, 2021

Updated. This still needs testing and I'm wondering how I'll to do that thinking

Hmm probably not worth trying to get a CI test for this. For manual testing, it looks like libdnf already logs debug output when it sets LRO_ONETIMEFLAG, so you should be able to use that: https://github.com/rpm-software-management/libdnf/blob/9a0e17562b19586b3ffa70fa93eb961b558794c7/libdnf/repo/Repo.cpp#L1132.

Edit: You can see those log messages with G_MESSAGES_DEBUG:

[root@cosa-devsh ~]# cat /etc/systemd/system/rpm-ostreed.service.d/override.conf
[Service]
Environment=G_MESSAGES_DEBUG=all
[root@cosa-devsh ~]# rpm-ostree install ltrace
...
[root@cosa-devsh ~]# journalctl --grep countme
-- Logs begin at Wed 2021-04-14 15:06:42 UTC, end at Wed 2021-04-14 15:13:12 UTC. --
Apr 14 15:08:32 cosa-devsh rpm-ostree[1189]: countme: no event for updates: budget to spend: 2
Apr 14 15:08:38 cosa-devsh rpm-ostree[1189]: countme: no event for fedora: budget to spend: 1

(Obviously with this PR, you shouldn't see any countme messages in the journal output.)

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Fine by me to merge as is if you've done the manual testing.

jlebon's suggestion is one approach; another may be for us to test via a custom webserver. But IMO not a blocker to merge this.

@travier
Copy link
Member Author

travier commented Apr 16, 2021

Did the testing and this one is working as expected. Had countme mentions before, none after:

-- Logs begin at Fri 2021-04-16 12:13:55 UTC, end at Fri 2021-04-16 17:38:31 UTC. --
Apr 16 12:15:42 cosa-devsh rpm-ostree[1383]: countme: no event for updates: budget to spend: 3
Apr 16 12:15:45 cosa-devsh rpm-ostree[1383]: countme: no event for fedora: budget to spend: 1
Apr 16 13:15:18 cosa-devsh rpm-ostree[1751]: countme: no event for updates: budget to spend: 2
Apr 16 13:15:20 cosa-devsh rpm-ostree[1751]: countme: event triggered for fedora: bucket 1
Apr 16 13:15:20 cosa-devsh rpm-ostree[1751]: Downloading: https://mirrors.fedoraproject.org/metalink?repo=fedora-33&arch=x86_64&countme=1
Apr 16 13:15:20 cosa-devsh rpm-ostree[1751]: Transfer finished: https://mirrors.fedoraproject.org/metalink?repo=fedora-33&arch=x86_64 (Effective url: https://>
Apr 16 13:24:50 cosa-devsh rpm-ostree[2306]: countme: no event for updates: budget to spend: 1
Apr 16 13:24:52 cosa-devsh rpm-ostree[2306]: countme: no event for fedora: window already counted
-- Reboot --
-- Reboot --

Used a combination of:

rpm-ostree cleanup -m
rpm-ostree refresh-md
rpm-ostree install foo ...

@cgwalters cgwalters merged commit 19e40a8 into coreos:master Apr 16, 2021
@travier travier deleted the nocountme branch April 19, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants