PR#1447: handle deleted tags in kojira - koji

koji

#1447 handle deleted tags in kojira

Merged 4 years ago by mikem. Opened 4 years ago by tkopecek.

tkopecek/koji issue1298 into master

Tomas Kopecek • 4 years ago

1e7640a

util/kojira

file modified

		`@@ -341,6 +341,7 @@`
		`self.logger.debug("Reading current repo data")`
		`repodata = self.session.getActiveRepos()`
		`self.logger.debug("Repo data: %r" % repodata)`
		`+`
		`for data in repodata:`
		`repo_id = data['id']`
		`repo = self.repos.get(repo_id)`
		`@@ -354,6 +355,9 @@`
		`self.logger.info('Found repo %s, state=%s'`
		`%(repo_id, koji.REPO_STATES[data['state']]))`
		`self.repos[repo_id] = ManagedRepo(self, data)`
		`+ if not getTag(repo.tag_id) and not repo.expired():`
		`+ self.logger.info('Tag %d for repo %d disappeared, expiring.' % (repo.tag_id, repo_id))`
		`+ repo.expire()`
		`if len(self.repos) > len(repodata):`
		`# This shouldn't normally happen, but might if someone else calls`
		`# repoDelete or similar`

tkopecek commented 4 years ago

Fixes: https://pagure.io/koji/issue/1298

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

4 years ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

4 years ago

mikem commented 4 years ago

Seems odd to grab all these taginfos each pass and only keep the existence. We probably ought to populate tag_cache when we do this.

rebased onto 1e7640a

4 years ago

tkopecek commented 4 years ago

Ah, missed, that local getTag already does reasonable caching for this usecase. Simplified.

Metadata Update from @tkopecek:
- Pull-request untagged with: testing-done

4 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

4 years ago

dgregor commented 4 years ago

@tkopecek to check and make sure there aren't any other cases that need updating as well for this

ktdreyer commented 4 years ago

(iirc kojiweb also crashes if you delete a tag)

tkopecek commented 4 years ago

@dgregor @mikem Issue I was talking about is #1064 (PR #1063) - I thinks we can drop that one, as I'm not sure where I've encountered it. I suspect, that it was only in my dev environment, so caused by dev process and it shouldn't appear in real world.

@ktdreyer Which page suffers from this / can you file a separate issue?

mikem commented 4 years ago

Side note: #1506 is a tangentially related follow-up

mikem commented 4 years ago

Traceback (most recent call last):
  File "/usr/sbin/kojira", line 1045, in <module>
    main(options, session)
  File "/usr/sbin/kojira", line 823, in main
    repomgr.readCurrentRepos()
  File "/usr/sbin/kojira", line 358, in readCurrentRepos
    if not getTag(repo.tag_id) and not repo.expired():
AttributeError: 'NoneType' object has no attribute 'tag_id'

mikem commented 4 years ago

This fixes the above and one other issue. (plus a bonus unrelated logging fix than came up in testing)

https://github.com/mikem23/koji-playground/commits/pagure/pr/1447

The only way I've been able to trigger the condition is to delete build tags while kojira is stopped and then start it. In that case, things seem to work fine. Deleting them while kojira is running seems to be handled elsewhere, but I suppose there is likely a race.

tkopecek commented 4 years ago

:thumbsup:

mikem commented 4 years ago

Commit a1be473 fixes this pull-request