Skip to content

(FACT-654) Don't try to resolve Ldom facts on non-SPARC systems#783

Closed
whopper wants to merge 1 commit intopuppetlabs:masterfrom
whopper:ldom_11.2_fix
Closed

(FACT-654) Don't try to resolve Ldom facts on non-SPARC systems#783
whopper wants to merge 1 commit intopuppetlabs:masterfrom
whopper:ldom_11.2_fix

Conversation

@whopper
Copy link

@whopper whopper commented Sep 4, 2014

In Solaris 11.2, Oracle introduced a rewrite of virtinfo which
supports both x86 and SPARC, whereas the old version only supported
SPARC. Since Ldoms are a SPARC-only feature, we now need to confine
the ldom facts to only resolve on SPARC based platforms.

Note that this information comes from community member Kristina Tripp,
who provided the necessary details on the new virtinfo implementation.

@whopper
Copy link
Author

whopper commented Sep 4, 2014

Note: though community members have mentioned that the behavior of virtinfo in SPARC systems should not have changed, we'd like to test this change out on a SPARC Solaris 11.2 system before merging.

@puppetcla
Copy link

CLA signed by all contributors.

Copy link

Choose a reason for hiding this comment

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

Nit: maybe split onto multiple lines, e.g. so it shows up w/o right scrolling in github :)

@kylog
Copy link

kylog commented Sep 9, 2014

Definitely test on an 11.2 sparc machine, but beyond that 👍

In Solaris 11.2, Oracle introduced a rewrite of virtinfo which
supports both x86 and SPARC, whereas the old version only supported
SPARC. Since Ldoms are a SPARC-only feature, we now need to confine
the ldom facts to only resolve on SPARC based platforms.

Note that this information comes from community member Kristina Tripp,
who provided the necessary details on the new virtinfo implementation.
@enpointe
Copy link

Tested this out on a few Sparc setup running 11.2 (Sparc Virtual, T2000, and T4 LDOM). No problems seen

@enpointe
Copy link

bundler exec rspec test on Sparc bare metal passes with

1673 examples, 0 failures, 4 pending

I have seen one error message being generated when running facter now twice on sparc hardware. Not sure whether this is related to this change or not. I'm not seeing this message on facter 2.1.0

Timed out after 6 seconds while resolving fact='virtual', resolution=''

@whopper
Copy link
Author

whopper commented Sep 25, 2014

@enpointe thanks a ton for running this through on SPARC! As for that 'virtual' fact error, could we have you run facter --trace on that machine and paste what you see? My best bet is that the error is coming from the SunOS confined 'virtual' fact, which has a timeout of 6 seconds built in.

It looks like that change was actually added back in February, but was lost from our codebase until we merged our Facter 2 branch back into master this Summer.

@enpointe
Copy link

I can't reproduce that complaint with any regularity. I'll see if I can reproduce it though. Like I said I've only seen it twice but I thought I should mention it as I saw that error message was getting generated by core/resolvable.rb.
BTW, 'bundle exec rspec' tests aren't clean on a LDOM. There's obviously some additional work needed for facter on a sparc LDOM. The tests though are clean when run on a bare SPARC hardware and x86 hardware.

@whopper
Copy link
Author

whopper commented Sep 25, 2014

Interesting! Thanks for the heads up. We're hoping to get some testing going on in SPARC and in LDOMs soon to sort these sorts of things out. Thanks again for your help!

@whopper whopper changed the title (Don't Merge)(FACT-654) Don't try to resolve Ldom facts on non-SPARC systems (FACT-654) Don't try to resolve Ldom facts on non-SPARC systems Sep 26, 2014
@whopper
Copy link
Author

whopper commented Sep 26, 2014

I'm closing this out to retarget the change at stable.

@whopper whopper closed this Sep 26, 2014
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.

4 participants