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

Add AIX Support to the collector #1652

Open
wants to merge 14 commits into
base: release/v1.60.0
Choose a base branch
from
Open

Add AIX Support to the collector #1652

wants to merge 14 commits into from

Conversation

Dylan-M
Copy link
Collaborator

@Dylan-M Dylan-M commented May 13, 2024

  • feat(AIX): = Enable building and deploying the collector for AIX

Proposed Change

Add AIX support to the collector.

Checklist
  • Changes are tested
  • CI has passed

jsirianni

This comment was marked as resolved.

@Dylan-M
Copy link
Collaborator Author

Dylan-M commented May 13, 2024

I think we should add dedicated builds and nFPM steps for AIX, instead of including it in the existing builds and nFPM. Specifically, I think we should avoid building deb packages.

This doesn't even create AIX packages currently, as it is not currently supported by nfpm. I have open issues to have support added. I just wanted to be ahead of it.

Once/if it is supported, it would only build rpm packages anyway, just like it only builds an msi for Windows. Seems like a lot of unneeded duplication of code to me, making it harder to maintain.
goreleaser/nfpm#821
goreleaser/goreleaser#4853

AIX differs enough, it might be benefit from its own pre and post scripts instead of expanding on the Linux scripts. This would allow us to focus on testing AIX instead of looking for regression on linux, with respect to the script changes.

That would be easy enough to do, but the changes are so simple, is it truly worth it? These scripts in general are super simple.

@Dylan-M
Copy link
Collaborator Author

Dylan-M commented May 14, 2024

Since the rpm portion doesn't currently work, I've restored the pre/post install scripts to what we had before. We can deal with that if the nfpm tooling adds AIX support.

config/example.yaml Outdated Show resolved Hide resolved
factories/exporters.go Outdated Show resolved Hide resolved
factories/processors_aix.go Show resolved Hide resolved
@Dylan-M Dylan-M force-pushed the aix branch 2 times, most recently from edd34a5 to 1891329 Compare May 14, 2024 22:00
@Dylan-M Dylan-M changed the base branch from release/v1.51.0 to release/v1.52.0 May 15, 2024 23:16
@Dylan-M Dylan-M force-pushed the aix branch 2 times, most recently from 71930de to 99e2834 Compare May 18, 2024 16:36
@Dylan-M Dylan-M requested a review from dpaasman00 as a code owner May 18, 2024 16:36
factories/processors_aix.go Outdated Show resolved Hide resolved
factories/exporters_aix.go Outdated Show resolved Hide resolved
factories/processors_aix.go Outdated Show resolved Hide resolved
factories/receivers_aix.go Show resolved Hide resolved
updater/internal/path/path_unix.go Outdated Show resolved Hide resolved
@Dylan-M Dylan-M force-pushed the aix branch 2 times, most recently from f21cdf1 to 6e96f2a Compare May 28, 2024 23:50
@Dylan-M Dylan-M changed the base branch from release/v1.52.0 to release/v1.53.0 June 3, 2024 13:19
@Dylan-M Dylan-M force-pushed the aix branch 3 times, most recently from 37e0fa8 to 5dba843 Compare June 7, 2024 16:07
Copy link
Member

@jsirianni jsirianni left a comment

Choose a reason for hiding this comment

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

@Dylan-M we should remove Corbin from this PR, if you can.

factories/receivers_aix.go Show resolved Hide resolved
@Dylan-M Dylan-M changed the base branch from release/v1.53.0 to release/v1.54.0 June 12, 2024 19:36
@Dylan-M Dylan-M added the enhancement New feature or request label Jun 20, 2024
@Dylan-M
Copy link
Collaborator Author

Dylan-M commented Jun 20, 2024

@jsirianni We aren't going to be able to vet the list of receivers any deeper than we currently have due to expanded list of customers potentially needing it.

@Dylan-M Dylan-M changed the base branch from release/v1.55.0 to release/v1.57.0 July 31, 2024 23:26
@Dylan-M Dylan-M changed the base branch from release/v1.57.0 to release/v1.58.0 August 8, 2024 15:17
@Dylan-M Dylan-M changed the base branch from release/v1.58.0 to release/v1.59.0 August 27, 2024 12:55
@Dylan-M
Copy link
Collaborator Author

Dylan-M commented Aug 27, 2024

@ryancgoins @jsirianni @dpaasman00 @BinaryFissionGames

Hey gents, I've just refreshed this PR this morning to bring it up to the current "default" release branch. I know the hardware isn't ready yet (yes, we have an actual Power server now that will run AIX), but I'm trying to be as prepped for that as possible ahead of time.

Were there any outstanding comments that need addressed beforehand? I know we had some discussion around changing some of the scripts to where AIX has its own unique ones instead of shared. Is that still a concern?

@BinaryFissionGames
Copy link
Member

@ryancgoins @jsirianni @dpaasman00 @BinaryFissionGames

Hey gents, I've just refreshed this PR this morning to bring it up to the current "default" release branch. I know the hardware isn't ready yet (yes, we have an actual Power server now that will run AIX), but I'm trying to be as prepped for that as possible ahead of time.

Were there any outstanding comments that need addressed beforehand? I know we had some discussion around changing some of the scripts to where AIX has its own unique ones instead of shared. Is that still a concern?

No high level concerns here. I'm fine with the script being shared between linux and AIX (it is called install_unix.sh after all).

@Dylan-M Dylan-M changed the base branch from release/v1.59.0 to release/v1.60.0 September 3, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants