Fixes #31545: Add status of Rails cache to Ping API

[ehelms]

Output from the API in the current format:

    "results": {
        "foreman": {
            "database": {
                "active": true,
                "duration_ms": "0"
            },
            "cache": {
                "type": "redis_cache_store",
                "servers": {
                    "0": {
                        "status": "ok",
                        "duration_ms": "0"
                    },
                    "1": {
                        "status": "FAIL",
                        "duration_ms": "1"
                    }
                }
            }
        },

Some considerations:

• Multiple Redis servers can be configured for cache
• The servers can independently fail and caching continues to work
• We do not want to leak too much information into the Ping API since it is unauthenticated

[theforeman-bot]

Issues: #31545

[ekohl]

Do we want to provide an overall OK/FAIL? I've been thinking about a cache set (with a short TTL) to test it end-to-end. Could be a potential attack vector though.

[ehelms]

I've done a round of updates here and found that #9857 is required to not get errors.

[ehelms]

When using a non-redis cache, do you feel that the response should be:

empty

{:foreman=>
    {
         :database=>{:active=>true, :duration_ms=>0},
         :cache=>{}
}

not present

{:foreman=>
   {
         :database=>{:active=>true, :duration_ms=>0}
},

[evgeni]

I think I prefer either completely omitting cache or returning a generic {status: OK} in there for the non-redis cases, but wouldn't block on either implementation.

[ShimShtein]

I see that the type property is missing from the final version. Ideally I think there should be at least the type, to indicate that the cache is enabled and what backend it uses.

So I think it should be an empty node, since we do know that the cache should be monitored, but we can't tell anything about its status. For me, missing node means that the status doesn't supply information about cache at all.

[ehelms]

I see that the type property is missing from the final version. Ideally I think there should be at least the type, to indicate that the cache is enabled and what backend it uses.

This was removed originally to avoid leaking information to a non-authenticated API endpoint.

[ofedoren]

Also, this would be nice if we'd have a follow-up PR to hammer https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/ping.rb :)

[ehelms]

Also, this would be nice if we'd have a follow-up PR to hammer https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/ping.rb :)

What is the needed follow up? I just assumed hammer outputs whatever the API sends. I'm sensing my assumption is incorrect.

[ofedoren]

Also, this would be nice if we'd have a follow-up PR to hammer https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/ping.rb :)

What is the needed follow up? I just assumed hammer outputs whatever the API sends. I'm sensing my assumption is incorrect.

Well, if it was just a blob, it would be true, but since we use explicit fields there, one must be added. Didn't tests this PR against hammer though, so if you'll see the output then the follow-up is not needed.

[ehelms]

Once this is merged, I'll take a look at hammer. I think this PR is ready to go?

[evgeni]

After applying the fix this seems to work fine for me.

hammer is at least not confused by the new fields:

[root@centos8-stream-foreman-nightly foreman]# hammer status
Version:           3.9.0-develop
API Version:       v2
Database:          
    Status:          ok
    Server Response: Duration: 1ms
Plugins:           
 1) Name:    foreman_puppet
    Version: 6.1.0
Smart Proxies:     
 1) Name:     centos8-stream-foreman-nightly.tanso.example.com
    Version:  3.9.0-develop
    Status:   ok
    Features: 
     1) Name:    puppetca
        Version: 3.9.0
     2) Name:    puppet
        Version: 3.9.0
     3) Name:    logs
        Version: 3.9.0
Compute Resources:

[root@centos8-stream-foreman-nightly foreman]# hammer ping
database: 
    Status:          ok
    Server Response: Duration: 0ms

[evgeni]

Wonder if this works w/o rebase:

/packit build

[evgeni]

@ofedoren would you mind having a look with hammer eyes? ;-)

I had added

          field :cache, _('cache'), Fields::Label do
            field :servers, _('servers'), Fields::Collection do
              field :status, _('Status')
              field :duration_ms, _('Server Response')
            end
          end

to my hammer ping.rb and things do not look too crazy

[ofedoren]

to my hammer ping.rb and things do not look too crazy

well, ideally they shouldn't :) And something like that (ideally) I'd expect in hammer to be able to see the cache details in hammer output. 👍

[evgeni]

to my hammer ping.rb and things do not look too crazy

well, ideally they shouldn't :) And something like that (ideally) I'd expect in hammer to be able to see the cache details in hammer output. 👍

theforeman/hammer-cli-foreman#622