epacke commented 5 years ago

Fixing bug with the direct links to the pool section not working
Fixing bug with the certificate table always showing direct links to the admin UI
Adding padding to sections that is not using data tables
Adding a more consistent style to the export buttons
Removed the optional export features since buttons are added by default anyway

timriker commented 5 years ago

The "Export to CSV" and the CSV buttons export different data. The "Export to CSV" I think was alway all columns and rows, where the CSV button exports the currently enabled columns and the current search result rows. Is there anything from the old "Export to CSV" that we want to keep?

timriker commented 5 years ago

The greyed out buttons on the "Virtual Servers" tab are shorter than on the other tabs. The new export button color looks great!

epacke commented 5 years ago

The greyed out buttons on the "Virtual Servers" tab are shorter than on the other tabs. The new export button color looks great!

I confess. I saw it, but I was too lazy to fix it. Thanks for pointing it out and making me a better person. :joy:

Replaced column toggles in VS server section to match the other sections
Replaced the Reset filters button with a DataTables button
Fixed a bug with the reset button in the certificate list section
Fixed a (new?) bug with the pool expansion auto expanding when the search is empty.

Check it out again?

epacke commented 5 years ago

The "Export to CSV" and the CSV buttons export different data. The "Export to CSV" I think was alway all columns and rows, where the CSV button exports the currently enabled columns and the current search result rows. Is there anything from the old "Export to CSV" that we want to keep?

Ah, I did not know that they were different. I replaced the action in the CSV export in the VS view with the old export. Consistent looks, but same export as the old export. Sounds good?

epacke commented 5 years ago

I have recently changed jobs and stopped working extra hours so I will put in some more energy here now. Will test the beta version some more and iron out some more kinks. Then we're approaching beta going live.

I'm thinking to replace the pool icon with something else. Also had the same idea since I appreciate the pun too, but the rest of the report looks more professional. What do you think?

timriker commented 5 years ago

"Reset Filters" on "Virtual Servers" does not clear column search text, but does seem to clear the search results?

timriker commented 5 years ago

The hash is updated with a few &=value options with no names. I didn't ever add the column filters for each table to the hash like the old virtual server column code used to do. It might be nice to save as vs1 vs2 vs3 and p1 p2 p3 etc. or perhaps vs0 and ps0 :) Anyway, I was thinking short names to keep the url short with the hash applied. If you're looking for work, it would be nice if an editted column search had an "x" by it to clear the column search without clearing all searches. :)

timriker commented 5 years ago

Oh, and I'm fine with any icon changes you like, I just tried to find freely licensed icons that worked. :) I like the "pool" pun and "devices" load "balance"rs puns, but I'm not attached to them. Have you thought about handling active/active pairs with multiple traffic groups? We're not yet running that type of setup, but I may set that up on one of my sandbox pairs to see how the report handles it.

epacke commented 5 years ago

The hash is updated with a few &=value options with no names. I didn't ever add the column filters for each table to the hash like the old virtual server column code used to do. It might be nice to save as vs1 vs2 vs3 and p1 p2 p3 etc. or perhaps vs0 and ps0 :) Anyway, I was thinking short names to keep the url short with the hash applied. If you're looking for work, it would be nice if an editted column search had an "x" by it to clear the column search without clearing all searches. :)

Good catch! I fixed it so that it should work like it used to. Not sure what you mean by short link? Agree that an x would make sense to clear the column filter but I think I will save that for another day. :)

Does it look ok now?

Oh, and I'm fine with any icon changes you like, I just tried to find freely licensed icons that worked. :) I like the "pool" pun and "devices" load "balance"rs puns, but I'm not attached to them. Have you thought about handling active/active pairs with multiple traffic groups? We're not yet running that type of setup, but I may set that up on one of my sandbox pairs to see how the report handles it.

I have not tried active-active either. Up until now I have mostly written the report according to features that I have needed myself and since I have never encountered active-active in my whole career I figured it was not so important. :) In fact, I find that the concept as it is is a bit strange unless you're using 3 (N+1) or more devices in the cluster. I mean, running active-active doubles your capacity but at the same time you forfeit on redundancy as you can never be sure that one device can handle the full load.

Suppose there might be corner cases where it makes sense. Have you encountered any?

epacke commented 5 years ago

Do you want to add the code from your other PR to this PR?

timriker commented 5 years ago

We don't store search values in tabs other than in virtualservers. If we start storing search entries from all tabs, the hesh could get really long. So instead of:

mainsection=virtualservers&loadbalancer=lb-&name=vs_&ipport=10.&global_search=lb-

We could shorten to:

sec=vs&vs1=lb-&vs2=vs_&vs4=10.&vs0=lb-

Then perhaps add support for pools as p0-5 etc. so we can link to a given search in other tabs.

epacke commented 5 years ago

Found some issues and added some comments inline. The log looks much nicer. Well done!

epacke commented 5 years ago

I can see now that I did not submit my review. Sorry, I am too used to Bitbucket. :)

timriker commented 5 years ago

One error array just has the messages, and one has the objects with date/time/severity. The other could be removed, but the email notification would need to pull from the formatted one. I was lazy and kept them both. The log() call before log() is declared worked for me. Perhaps this varies based on powershell version? Getting everything to use log() mean that I could comment out the filter, and see everything in loggederrors.json in the report.

epacke commented 5 years ago

One error array just has the messages, and one has the objects with date/time/severity. The other could be removed, but the email notification would need to pull from the formatted one. I was lazy and kept them both. The log() call before log() is declared worked for me. Perhaps this varies based on powershell version? Getting everything to use log() mean that I could comment out the filter, and see everything in loggederrors.json in the report.

Try running the script in a fresh terminal to avoid functions declared in previous sessions to poison the namespace? Also, try to only change the minimal amount of configuration to run the script. It throws errors for me at least.

Which PS version do you have? Here's mine:

PS C:\Users\Patrik\Documents\BigIPReport> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.17763.503
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17763.503
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

timriker commented 5 years ago

the "c" is your function. I just copied the initializer down.

timr@A-LE2009442:~$ powershell.exe \$PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.18362.113
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.18362.113
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

I'm normally running under a WSL/Ubuntu prompt so I start a new powershell.exe when I run the script. I just tried that again, and got errors. Hmm. Perhaps I tested it under vscode before commit? Not sure.

epacke commented 5 years ago

the "c" is your function. I just copied the initializer down.

Could have sworn it looked familiar. The function is taken directly from SO. :)

epacke commented 5 years ago

Fixed the error with the log function
Updated the icons for pools and device overview
Added a favicon
Added Z in front of virtual edition licenses

epacke / BigIPReport

Minor fixes and css improvements #129

mainsection=virtualservers&loadbalancer=lb-&name=vs_&ipport=10.&global_search=lb-

sec=vs&vs1=lb-&vs2=vs_&vs4=10.&vs0=lb-