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 cisco IMC MIBs #1293

Merged
merged 19 commits into from
Dec 3, 2024
Merged

Conversation

ranmalka
Copy link
Contributor

Cisco IMC MIBS to monitor Cisco CUCS server IMC (parallel to HPE-ILO)

@SuperQ
@bastischubert
@RichiH
Signed-off-by: ran [email protected]

@ranmalka
Copy link
Contributor Author

@bastischubert I reduced it to 3 metrics instead of 3000 lines, hope its ok now

snmp.yml Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
@ranmalka
Copy link
Contributor Author

I would like to get your approval :)
@bastischubert @SuperQ

Thanks,

@bastischubert
Copy link
Member

@ranmalka have you checked my last comments? after that i think we're ready to merge

@ranmalka
Copy link
Contributor Author

@ranmalka have you checked my last comments? after that i think we're ready to merge

@bastischubert do you mean to put the files directly in the mibs location? so yes that's what I did

@bastischubert
Copy link
Member

@ranmalka i left a few comments that still need to be adressed with the Makefile

@ranmalka
Copy link
Contributor Author

ranmalka commented Dec 1, 2024

@bastischubert conflicts have been fixed :)

@bastischubert
Copy link
Member

@ranmalka did you notice the comments i left and are still not yet addressed ? https://github.com/prometheus/snmp_exporter/pull/1293/files you should see them there.

  • i asked why you added the CISCO_FRU_URL which seems unused.
  • $(MIBDIR)/cisco_imc to $(MIBDIR)/.cisco_imc still needs to be addressed

everything else already looks good and ready to get merged 👍

@ranmalka
Copy link
Contributor Author

ranmalka commented Dec 1, 2024

@bastischubert CISCO_FRU_URL used by 1.3.6.1.4.1.9.9.719.1.1.1.1.11 it didn't work before that

why do I need to add .cisco_imc folder? I added all the mibs files into the /mibs directory

Signed-off-by: ran <[email protected]>
@bastischubert
Copy link
Member

bastischubert commented Dec 3, 2024

@ranmalka

CISCO_FRU_URL used by 1.3.6.1.4.1.9.9.719.1.1.1.1.11 it didn't work before that

i checked, you only need those 4 mibs to get config working:
CISCO-UNIFIED-COMPUTING-FAULT-MIB.my
CISCO-UNIFIED-COMPUTING-MIB.my
CISCO-UNIFIED-COMPUTING-STORAGE-MIB.my
CISCO-UNIFIED-COMPUTING-TC-MIB.my

why do I need to add .cisco_imc folder? I added all the mibs files into the /mibs directory

basically to prevent the download on every run of make mibs - plus putting that marker in a hidden file instead of a directory.

Signed-off-by: ran <[email protected]>
@ranmalka
Copy link
Contributor Author

ranmalka commented Dec 3, 2024

@bastischubert

CISCO_FRU_URL used by 1.3.6.1.4.1.9.9.719.1.1.1.1.11 it didn't work before that

i checked, you only need those 4 mibs to get config working: CISCO-UNIFIED-COMPUTING-FAULT-MIB.my CISCO-UNIFIED-COMPUTING-MIB.my CISCO-UNIFIED-COMPUTING-STORAGE-MIB.my CISCO-UNIFIED-COMPUTING-TC-MIB.my

I also added cisco-TCI and CISCO-SMI to your list, looks fine (tests passed)

basically to prevent the download on every run of make mibs - plus putting that marker in a hidden file instead of a directory.

Now is that ok?

let me know what else is needed

Copy link
Member

@bastischubert bastischubert left a comment

Choose a reason for hiding this comment

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

LGTM

snmp.yml Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
generator/Makefile Outdated Show resolved Hide resolved
@bastischubert bastischubert merged commit e9a9866 into prometheus:main Dec 3, 2024
6 checks passed
@bastischubert
Copy link
Member

@ranmalka thanks for your patience and your contribution 🚀

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.

3 participants