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

Update scripts, add konflux automation files #1

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

lsm5
Copy link
Collaborator

@lsm5 lsm5 commented Sep 19, 2024

This commit updates the setup script used for building the container image.

It also adds 2 files:

  1. rpms.in.yaml: contains the packages of concern along with the dnf repos containing rpm builds, sources and metadata of said packages
  2. rpms.lock.yaml: this file is generated by running the rpm-lockfile-prototype tool on rpms.in.yaml. Konflux-CI will run a Renovate job that will update rpms.lock.yaml whenever any of the packages mentioned in rpms.in.yaml gets updated in the DNF repos.

This commit combined with a quay.io trigger to build new images on every github push will enable a fully auomated workflow for updated PQ container image.

This commit updates the setup script used for building the container
image.

It also adds 2 files:
1. `rpms.in.yaml`: contains the packages of concern along with the dnf
    repos containing rpm builds, sources and metadata of said packages
2. `rpms.lock.yaml`: this file is generated by running the
   `rpm-lockfile-prototype` tool on rpms.in.yaml. Konflux-CI will run
   a Renovate job that will update rpms.lock.yaml whenever any of the
   packages mentioned in rpms.in.yaml gets updated in the DNF repos.

This commit combined with a quay.io trigger to build new images on every
github push will enable a fully auomated workflow for updated PQ
container image.

Signed-off-by: Lokesh Mandvekar <[email protected]>
@lsm5
Copy link
Collaborator Author

lsm5 commented Sep 19, 2024

Hi @ralphbean, my main concern here is to have konflux / renovate automatically create update PRs for rpms.lock.yaml whenever dnf repos get updated. Does it suffice to have the konflux github added to the repo (done already) or do we need tekton pipelines enabled via #2 ?

Until konflux support for building images to quay is mature enough, we'll be using quay's default github triggers for automated image builds.

@lsm5
Copy link
Collaborator Author

lsm5 commented Sep 19, 2024

@ralphbean , another concern I have is that konflux-fedora seems to create PRs based on older commits resulting in duplicate PRs. See lsm5/pqc#24 . It's showing me a diff that shouldn't exist. The destination branch is not main in case that matters. You'll see 2 other merged PRs, 23 and 22 in https://github.com/lsm5/pqc/pulls?q=is%3Apr+is%3Aclosed with the same diff.

@lsm5 lsm5 marked this pull request as ready for review September 20, 2024 10:02
@sahanaprasad07
Copy link
Contributor

LGTM

@sahanaprasad07 sahanaprasad07 merged commit 8f8d63b into QUBIP:main Sep 20, 2024
@lsm5 lsm5 deleted the automation branch September 20, 2024 10:53
@ralphbean
Copy link

Does it suffice to have the konflux github added to the repo (done already) or do we need tekton pipelines enabled via #2 ?

I don't think you need to merge #2 in order to have the renovate instance associated with konflux-fedora update your rpm lock file. You do need to have a Component registered with konflux-fedora for QUBIP/pq-container though in konflux-fedora.

@ralphbean
Copy link

ralphbean commented Sep 20, 2024

Another concern I have is that konflux-fedora seems to create PRs based on older commits resulting in duplicate PRs.

Trying to make sure I understand. These three PRs:

... were all generated with essentially the same diff, same commit. Duplicates. That's definitely erroneous.

Can we close lsm5/pqc#24 and monitor to see if renovate submits the same change again?


@cit1zen these PRs originate from the fedora instance of konflux that people are experimenting with. Can you imagine why this might have happened?

I can see from the deployment of mintmaker (which is not yet open) that the following refs are deployed:

...
  - https://github.com/konflux-ci/mintmaker/config/default?ref=bc582566fb7289479284adf75f2c51c0d56b9207
  - https://github.com/konflux-ci/mintmaker/config/renovate?ref=bc582566fb7289479284adf75f2c51c0d56b9207

images:
  - name: quay.io/konflux-ci/mintmaker
    newName: quay.io/konflux-ci/mintmaker
    newTag: bc582566fb7289479284adf75f2c51c0d56b9207

@lsm5
Copy link
Collaborator Author

lsm5 commented Sep 23, 2024

You do need to have a Component registered with konflux-fedora for QUBIP/pq-container though in konflux-fedora.

@ralphbean For registering a component, is the process the same as Create Application -> Add Component ? Wouldn't that end up creating the same PR as #2 ?

@lsm5
Copy link
Collaborator Author

lsm5 commented Sep 23, 2024

Another concern I have is that konflux-fedora seems to create PRs based on older commits resulting in duplicate PRs.

Trying to make sure I understand. These three PRs:

... were all generated with essentially the same diff, same commit. Duplicates. That's definitely erroneous.

Can we close lsm5/pqc#24 and monitor to see if renovate submits the same change again?

I deleted the application over there especially after moving to this official repo, but let me give that another try on my fork. I'll merge a few PRs even if I see the same diff and see if there are repeated dups.

@lsm5
Copy link
Collaborator Author

lsm5 commented Sep 24, 2024

@ralphbean in the konflux-fedora application pipeline, I can login with my FAS but I can't add other users to my tenant using their FAS id. Looks like I'll need them to be signed into RHTAP. I think FAS account acceptance would be highly desirable. Also desirable would be tenant space for organizations with individual FAS accounts as admins / maintainers. Hope these 2 requests can be prioritized in the container-sig effort.

@lsm5
Copy link
Collaborator Author

lsm5 commented Oct 4, 2024

@ralphbean so it looks like we don't need #2 included if all we care about is updating the rpm lock file as evident in #4 . All we need is the service app added and the lock file.

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