-
Notifications
You must be signed in to change notification settings - Fork 497
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
(FACT-3119) RFC2863 operational state for network interfaces #2488
Conversation
Can one of the admins verify this patch? |
d9d4431
to
007e91f
Compare
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.
The code looks good to me, I think the one question is whether operational state
is too linux specific: https://docs.kernel.org/networking/operstates.html? I think the answer is no, since there's an SNMP MIB defined in the linked RFC https://datatracker.ietf.org/doc/html/rfc2863#section-6 and it shows up in non-Linux hosts too:
https://learn.microsoft.com/en-us/dotnet/api/system.net.networkinformation.operationalstatus?view=net-7.0
changes look good to me but i don't have any rights :) |
@jcpunk can you rebase on upstream and resolve the test failures? This seemed to resolve the linux ones, but there may be others: diff --git a/spec/facter/resolvers/linux/networking_spec.rb b/spec/facter/resolvers/linux/networking_spec.rb
index 21d9e6b43..4e42783d4 100644
--- a/spec/facter/resolvers/linux/networking_spec.rb
+++ b/spec/facter/resolvers/linux/networking_spec.rb
@@ -21,6 +21,8 @@ describe Facter::Resolvers::Linux::Networking do
allow(File).to receive(:exist?).with('/proc/net/if_inet6').and_return(true)
allow(Facter::Util::FileHelper).to receive(:safe_read)
.with('/proc/net/if_inet6', nil).and_return(load_fixture('proc_net_if_inet6').read)
+ allow(Facter::Util::FileHelper).to receive(:safe_read).with('/sys/class/net/lo/operstate', nil).and_return('unknown')
+ allow(Facter::Util::FileHelper).to receive(:safe_read).with('/sys/class/net/ens160/operstate', nil).and_return('up')
end
after do
@@ -68,6 +70,7 @@ describe Facter::Resolvers::Linux::Networking do
netmask6: 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff',
network: '127.0.0.0',
network6: '::1',
+ operational_state: 'unknown',
scope6: 'host'
},
'ens160' => {
@@ -88,6 +91,7 @@ describe Facter::Resolvers::Linux::Networking do
netmask6: 'ffff:ffff:ffff:ffff::',
network: '10.16.112.0',
network6: 'fe80::',
+ operational_state: 'up',
scope6: 'link'
}
}
@@ -158,6 +162,7 @@ describe Facter::Resolvers::Linux::Networking do
netmask6: 'ffff:ffff:ffff:ffff::',
network: '10.16.112.0',
network6: 'fe80::',
+ operational_state: 'up',
scope6: 'link'
}
end
@@ -207,7 +212,8 @@ describe Facter::Resolvers::Linux::Networking do
mac: '00:50:56:9a:61:46',
mtu: 1500,
netmask: '255.255.240.0',
- network: '10.16.112.0'
+ network: '10.16.112.0',
+ operational_state: 'up',
}
expect(networking_linux.resolve(:interfaces)['ens160']).to eq(expected)
@@ -297,8 +303,6 @@ describe Facter::Resolvers::Linux::Networking do
allow(File).to receive(:exist?).with('/proc/net/if_inet6').and_return(true)
allow(Facter::Util::FileHelper).to receive(:safe_read)
.with('/proc/net/if_inet6', nil).and_return(load_fixture('proc_net_if_inet6').read)
- allow(Facter::Util::FileHelper).to receive(:safe_read).with('/sys/class/net/lo/operstate').and_return('unknown')
- allow(Facter::Util::FileHelper).to receive(:safe_read).with('/sys/class/net/ens160/operstate').and_return('up')
end
it 'returns all the interfaces' do |
70f146a
to
348a5f6
Compare
I've rebased, hopefully that helps. |
348a5f6
to
ed0a1ca
Compare
ed0a1ca
to
386fedd
Compare
@jcpunk Could you rebase? |
Signed-off-by: Pat Riehecky <[email protected]>
386fedd
to
b21f669
Compare
Rebased |
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.
Ignore the rubocop error, I'll put up a PR to remove that check
Follow up PR #2607 |
This adds RFC2863 operational state information for Linux.
I confess I'll need help with the tests....