search

delphix / linux-pkg

Framework to build custom packages for the Delphix Appliance
Apache License 2.0
4 stars 31 forks source link

DLPX-87969 Modify kernel build to include Delphix's custom annotations #300

Closed palash-gandhi closed 1 year ago

palash-gandhi commented 1 year ago

Problem

https://github.com/delphix/linux-kernel-aws/pull/46 and it's side ports introduced a potential for merge conflicts in the base annotations file. If upstream changes that file, we are bound to hit a merge conflict when updating our repos because the `include` line is at the end of the file. Moving it to the top result in an unwanted configuration - where upstream's annotations overrides Delphix's annotations. We want Delphix's annotations to be processed at the very end.

Solution

Move Delphix's annotations file to this repo and remove them from the kernel repos. Then, at build time, import the annotations. Associated kernel repo PRs: https://github.com/delphix/linux-kernel-aws/pull/48 https://github.com/delphix/linux-kernel-azure/pull/31 https://github.com/delphix/linux-kernel-gcp/pull/32 https://github.com/delphix/linux-kernel-generic/pull/31 https://github.com/delphix/linux-kernel-oracle/pull/30

Testing Done

aws: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/7043/ azure: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/7044/ generic: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/7026/ gcp: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/7027/ oracle: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/7028/ All of my Blackbox test runs failed because the upgrade image size dropped by more than 10%: Before my change: ``` $ aws s3 ls s3://snapshot-de-images/builds/jenkins-ops/appliance-build/release/post-push/74/upgrade-artifacts/internal-qa.upgrade.tar 2023-09-14 22:18:35 7269447680 internal-qa.upgrade.tar ``` After my changes (verified all 5 upgrade images from these runs): ``` $ aws s3 ls s3://dev-de-images/builds/jenkins-selfservice/appliance-build/develop/pre-push/1475/upgrade-artifacts/internal-qa.upgrade.tar 2023-09-14 21:09:12 5589022720 internal-qa.upgrade.tar ```

Implementation

I had to add the following 2 annotations to make `updateconfigs` fully non-interactive: ``` CONFIG_GPIO_BT8XX policy<{'amd64': 'n', 'arm64': 'n'}> CONFIG_INTEL_ATOMISP2_PM policy<{'amd64': 'n', 'arm64': 'n'}> ``` because while running `updateconfigs`, I got ``` * * Restart config... * * * PCI GPIO expanders * AMD 8111 GPIO driver (GPIO_AMD8111) [M/n/y/?] m BT8XX GPIO abuser (GPIO_BT8XX) [N/m/y/?] (NEW) ^Cmake[3]: *** [../scripts/kconfig/Makefile:77: syncconfig] Interrupt ``` and ``` * * Restart config... * * * X86 Platform Specific Device Drivers * X86 Platform Specific Device Drivers (X86_PLATFORM_DEVICES) [Y/n/?] y ... ... Intel AtomISP v2 dummy / power-management driver (INTEL_ATOMISP2_PM) [N/m/y/?] (NEW) ^Cmake[3]: *** [../scripts/kconfig/Makefile:77: syncconfig] Interrupt ```
palash-gandhi commented 1 year ago

I'm not necessarily convinced this is needed.. have we gotten multiple merge conflicts already? or only one?

We have only gotten 1 merge conflict so far. We can wait and observe if this change is really needed.

With that said, I'm also not opposed to this change, from a design perspective...

If we want to go with this approach, I think I'd recommend we also look into dynamically creating the debian.delphix/config/annotations file too.. what do you think?

That's also an option, yes. We could move the annotations file to linux-pkg or have logic that creates the file right before building the kernel. But I figured it's easier for the configuration to live in the repo itself. Generating the config has the same pros & cons as the 2 options listed here. Or did you have something else in mind?

prakashsurya commented 1 year ago

I think I actually like option 2, over what you have here.. and then, merging that with what I mentioned in my comment.. we could change -f debian.delphix/config/annotations to point to a single file that lives in this repository (linux-pkg), and is used for all of our kernel repositories..

Obviously, that means the config we use to override must apply to all the different platforms, but I think the file is the same today, so that would work. If we find that doesn't work in the future, we can think about how to extend it to work for platform specific overrides.