Added Last Status information for Container

[stevejr]

• This update will support a last status of terminated and display the information for the terminated state.

Fixes: #926

Signed-off-by: Steve Richards srichards@mirantis.com

[nevalla]

so this is covering running and waiting states?

[stevejr]

@nevalla - No, the lastState is only going to show terminated. Do we need the lastState to also show waiting as it can't be running as if a pod terminates and restarts it would be status of running and lastState of terminated.

[nevalla]

I would change this to {lastState &&. So not displaying even the title if Last status is not found.

[stevejr]

That was a typo. Corrected in next commit

[nevalla]

Please could you give an example, how these are looking after rendered?

[stevejr]

[Nokel81]

That is rather hard to read. @nevalla Would a table-like view be better for information like this?

[nevalla]

How about putting line breaks after exit code and timestamps, so the result would be:

terminated, Error (exit code: 255)
started at: 2020-10-12T10:19:20Z
finished at: 2020-10-13T15:49:23Z

Would it be any better?

[Nokel81]

That would be better, however then the comma after lastState scans oddly

[stevejr]

How about no comma and we just separate each item with a line break?

[stevejr]

@Nokel81 , @nevalla - with this logic:

renderLastState(lastState: string, status: IPodContainerStatus) {
    if (lastState === 'terminated') {
      return (
        <span>
          {lastState}<br/> 
          {_i18n._(t`Reason`)}: {status.lastState.terminated.reason} - {_i18n._(t`exit code`)}: {status.lastState.terminated.exitCode}<br/> 
          {_i18n._(t`Started at`)}: {status.lastState.terminated.startedAt}<br/> 
          {_i18n._(t`Finished at`)}: {status.lastState.terminated.finishedAt}<br/> 
        </span>
      )  
    }
  }

we get the following output - is this ok?

[nevalla]

LGTM

[Nokel81]

This is a decently complicated inline render. I personally would like to see it moved to its own function

[stevejr]

@Nokel81 - something like this:

renderLastState(lastState: string, status: IPodContainerStatus) {
    return (
      <span>
        {lastState ? `${lastState}, ` : ""}
        {lastState === 'terminated' ? `${status.lastState.terminated.reason} (${_i18n._(t`exit code`)}: ${status.lastState.terminated.exitCode}), 
          ${_i18n._(t`started at`)}: ${status.lastState.terminated.startedAt}, ${_i18n._(t`finished at`)}: ${status.lastState.terminated.finishedAt}` : ''}
      </span>
    );
  }

and the calling line:

{lastState &&
        <DrawerItem name={<Trans>Last Status</Trans>}>
          {this.renderLastState(lastState, status)}
        </DrawerItem>

Wasn't sure if the DrawerItem should also be moved - thought it was better to leave it in the main body for ease of reading.

[Nokel81]

That could work, I would have the drawer item in the separate function but that is more of a personal preference.

[stevejr]

I can't see any other examples where we do have the DrawerItem in the function so will leave as currently committed.

[Nokel81]

I think it is good now