-
Notifications
You must be signed in to change notification settings - Fork 57
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
(BKR-937) Add support for installing AIX 7 agent on 7.2 #54
Conversation
Can one of the admins verify this patch? |
Refer to this link for build results (access rights to CI server needed): |
lib/beaker-pe/install/pe_utils.rb
Outdated
@@ -289,7 +289,11 @@ def fetch_pe(hosts, opts) | |||
# @param [Host] host The host to install pacakges for | |||
# @api private | |||
def deploy_frictionless_to_master(host) | |||
klass = host['platform'].gsub(/-/, '_').gsub(/\./,'') | |||
platform = host['platform'] | |||
if platform == "aix-7.2-power" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@branan can you remind me why we are making this change for 7.2 AIX machines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have separate AIX 7.2 builds (they use 7.1). this means that when we classify the master for pe_repo
, a 7.2 agent needs the 7.1 classification. The rest of the difference in the install are handled by pe_repo
itself knowing how to install the 7.1 binaries on 7.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you leave a comment with this code change to state this 7.2 needs uses 7.1 classification? I also believe that you had mentioned plans to improve the AIX installers to not care about the AIX version, which could also result in this cod eventually being removed, you know, in the future. 👾
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can add a comment to that effect. I will definitely be in here messing with this again in the agent 2.0 timeframe.
To avoid unnecessarily expanding our build times, our initial support for AIX 7.2 is through the existing 7.1 package. This will be replaced with a single unified AIX build in the Agent 2.0 timeframe, but for now we need to add a little bit of special-case logic to handle 7.2 with existing packages.
7604741
to
9aada07
Compare
@tvpartytonight Added a comment as requested |
Refer to this link for build results (access rights to CI server needed): |
LGTM |
To avoid unnecessarily expanding our build times, our initial support
for AIX 7.2 is through the existing 7.1 package. This will be replaced
with a single unified AIX build in the Agent 2.0 timeframe, but for now
we need to add a little bit of special-case logic to handle 7.2 with
existing packages.