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

Implements linux packaging with nfpm #366

Merged
merged 15 commits into from
Oct 10, 2021
Merged

Implements linux packaging with nfpm #366

merged 15 commits into from
Oct 10, 2021

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Sep 1, 2021

This creates Debian and RPM packages on make dist and tests them on each commit.

Upon release, these are signed with an org secret.

fixes #345

@codefromthecrypt
Copy link
Contributor Author

@dio as I'll be offline at least another day, if you are interested in carrying this one on, it would be helpful. I mentioned in the PR description notes that basically we should make sure our GitHub actions can install and run the rpm and deb just like msi (and probably the more coherent way to do that is to refactor msi.yaml into a matrix test that also does rpm and deb). There may be some devil in details ex centos isn't an operating system choice here. You can see in one of the GitHub actions where we are using docker to run centos for this reason. If there isn't enough bread crumbs here to carry it on, feel free to let it sit, just might be nice to get into the next release and possibly not a huge reason for me to sit on it

@dio
Copy link
Collaborator

dio commented Sep 14, 2021

Sorry missed your comment. @codefromthecrypt I'll try to work on it. Take care, @codefromthecrypt!

@dio
Copy link
Collaborator

dio commented Sep 14, 2021

Original PR description from @codefromthecrypt:

WIP of #345

This needs to be manually tested, then automated tests on release, probably by renaming msi.yaml to packaging.yaml and making rpm and deb tests similar to how we do msi, except for ubuntu and centos

I already generated a key and sent it to the places and added as org secrets

  • Manually tested
  • Renaming msi.yaml to packaging.yaml

I added the following, for testing install/uninstall on Linux (re: making rpm and deb tests similar to how we do msi, except for ubuntu and centos).

  • verify script (install/uninstall)
  • job using verify script to verify install/uninstall on x86 (ubuntu (_amd64.deb), centos (_x86_64.rpm))
  • job using verify script to verify install/uninstall on aarch64 (ubuntu (_arm64.deb), fedora (_aarch64.rpm)), non-x86.

@dio dio changed the title WIP: implements linux packaging with nfpm Implement linux packaging with nfpm Sep 14, 2021
@dio dio marked this pull request as ready for review September 14, 2021 15:21
@dio
Copy link
Collaborator

dio commented Sep 14, 2021

@mathetake when you have time, please help us to review this. Thanks!

Copy link
Contributor Author

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

Thanks for giving this a try. I think my main feedback is around the inconsistency here, doing things differently to accomplish similar goals. That's really not a good idea as it adds even more distraction if merged.

One thing I'll ask always is to not make one-off changes to build config or really anything that's otherwise done consistently. Ex formatting one file completely different to the others doesn't help maintenance, and instead it makes the project look and feel sloppy. Please roll back the formatting or do it in another PR to all files.

Similarly it is unnecessary to deviate from how we test on centos depending on if we are using rpm vs just running func-e directly on e2e. Try to research how other things are done first for the same reason as formatting.. That will result in even more important consistency. Docker usually isn't a game we'd want to get into, and luckily between the container directive here and native runners on travis for arm, we really don't need to do most or any of explicit dockering.

I think the idea end result is a single linux package test matrix in the packaging and release yaml. arm64 + deb or rpm if tested only on commit with travis, even if only partially tested (ex deb only), is better than introducing a fairly gnarly GitHub action. If we did want to go the pure emulation route, and inherit the gnarly action we'd probably want to remove travis. Having one leg in each canoe is on the other hand worse.

In any case, if we wanted to completely change test approach that would be something we could do after doing things consistently first.

packaging/nfpm/verify.sh Outdated Show resolved Hide resolved
.github/workflows/packaging.yaml Outdated Show resolved Hide resolved
.github/workflows/packaging.yaml Outdated Show resolved Hide resolved
.github/workflows/packaging.yaml Outdated Show resolved Hide resolved
@dio
Copy link
Collaborator

dio commented Sep 14, 2021

I reverted my changes. And convert it to a draft. Obviously, I'm a thousand years behind in understanding what consistency in code means 😞. I will re-think before making any changes (or just wait until someone else has a better execution plan for this).

.github/workflows/release.yaml Outdated Show resolved Hide resolved
@dio dio mentioned this pull request Sep 19, 2021
@codefromthecrypt
Copy link
Contributor Author

one thing I can hope we can do is "be consistent and use the right tool for the job" I have made countless requests to default to practice that already exists here, as a lot of thought went into that. The second classification of requests were to find the simplest way out. Introducing more layers of technology (ex buildx, emu etc) is the opposite of this and generally how all projects end up a mess at some point and/or abandoned.

I'm not interested in stressing myself out repeating the same requests again and again, and hope that you can please focus on the smallest change first vs holding something hostage to a complex set of infrastructure changes. we have time to discuss changes but as I've noticed many times, the current status quo is not understood, and until it is, it isn't the right time to change it.

@codefromthecrypt
Copy link
Contributor Author

UPDATE:

GHA and Travis as of #383 run e2e tests on the matrix [ubuntu,centos]*[amd64,arm64]

NEXT STEP:

Rebase this and use the same centos image to run the rpm tests

@codefromthecrypt
Copy link
Contributor Author

UPDATE:

In order to have coherent builds across platforms and CI, Make needs to own GOROOT selection and configuration. #386 implements that.

NEXT ACTION:

Once the above is merged, complete GHA runner-style docker images, use them for e2e tests and ultimately here for RPM and Debian tests.

@codefromthecrypt
Copy link
Contributor Author

#387 will push the docker images we need, once finished

@codefromthecrypt
Copy link
Contributor Author

rebased and will rebase again after #392 and then finish it up

Adrian Cole and others added 13 commits October 8, 2021 12:51
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Dhi Aurrahman <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Adrian Cole added 2 commits October 8, 2021 13:59
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt codefromthecrypt changed the title Implement linux packaging with nfpm Implements linux packaging with nfpm Oct 9, 2021
@codefromthecrypt codefromthecrypt marked this pull request as ready for review October 9, 2021 00:11
@codefromthecrypt
Copy link
Contributor Author

ok this is as well tested as I can do without running a test release. I'll do that after merge

@codefromthecrypt codefromthecrypt merged commit 773f0fa into master Oct 10, 2021
@codefromthecrypt
Copy link
Contributor Author

thanks for the help on this @dio!

@codefromthecrypt
Copy link
Contributor Author

pulled the package signing because it never worked. the test scripts didn't verify, so they didn't notice. Make didn't export the variables, so signing was silently ignored until release which exported them #395

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.

Publish Linux system packages (debian and RPM)
2 participants