Skip to content

Conversation

adfost
Copy link
Contributor

@adfost adfost commented Jun 10, 2021

Screen Shot 2021-06-11 at 5 22 15 PM

setErrorSnackMessage(err);
});
}, [tenantNamespace, tenantName, podName, setErrorSnackMessage]);
loadInfo();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the content of loadInfo function inside this effect and remove the // eslint-disable-next-line react-hooks/exhaustive-deps. I think this change shouldn't go

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please review this

Comment on lines 205 to 214
api
.invoke(
"GET",
`/api/v1/namespaces/${tenantNamespace}/tenants/${tenantName}/pods/${podName}`
)
.then((res: string) => {
setLogLines(res.split("\n"));
})
.catch((err) => {
setErrorSnackMessage(err);
});
}, [tenantNamespace, tenantName, podName, setErrorSnackMessage]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the content of loadInfo function inside this effect and remove the // eslint-disable-next-line react-hooks/exhaustive-deps. I think this change shouldn't go

{" > "}
<Link
to={`/namespaces/${tenantNamespace}/tenants/${tenantName}`}
to={`/namespaces/${match.params["tenantNamespace"]}/tenants/${match.params["tenantName"]}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use tenantNamespace & tenantName variables

className={classes.breadcrumLink}
>
{tenantName}
{match.params["tenantName"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use tenantName var

{match.params["tenantName"]}
</Link>
{` > Pods > ${podName}`}
{` > Pods > ${match.params["podName"]}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use podName var

retval := []*models.TenantPod{}
for _, pod := range pods.Items {
var restarts int64
var restarts int64 = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check gofmt checks

@dvaldivia
Copy link
Collaborator

Can you attach a screenshot of how it looks?

@dvaldivia
Copy link
Collaborator

Can you sort the events in ascending order (older on top, newer at the bottom) and group them?

Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

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

Please fix all the warnings introduced on PodDetails.tsx

src/screens/Console/Tenants/TenantDetails/PodDetails.tsx
  Line 19:8:    'get' is defined but never used                      @typescript-eslint/no-unused-vars
  Line 31:3:    'Button' is defined but never used                   @typescript-eslint/no-unused-vars
  Line 32:3:    'IconButton' is defined but never used               @typescript-eslint/no-unused-vars
  Line 33:3:    'Menu' is defined but never used                     @typescript-eslint/no-unused-vars
  Line 34:3:    'MenuItem' is defined but never used                 @typescript-eslint/no-unused-vars
  Line 48:8:    'history' is defined but never used                  @typescript-eslint/no-unused-vars
  Line 49:10:   'LogMessage' is defined but never used               @typescript-eslint/no-unused-vars
  Line 50:10:   'IPolicyItem' is defined but never used              @typescript-eslint/no-unused-vars
  Line 51:10:   'TabPanel' is defined but never used                 @typescript-eslint/no-unused-vars
  Line 167:10:  'selectedTab' is assigned a value but never used     @typescript-eslint/no-unused-vars
  Line 167:23:  'setSelectedTab' is assigned a value but never used  @typescript-eslint/no-unused-vars
  Line 168:19:  'setLoading' is assigned a value but never used      @typescript-eslint/no-unused-vars
  Line 169:10:  'log' is assigned a value but never used             @typescript-eslint/no-unused-vars

@dvaldivia
Copy link
Collaborator

While doing kubectl get event I'm seeing the last event, 4 minutes ago but the UI is showing 6 hours ago

➜ k get event -n ns-2 --field-selector involvedObject.name=asdzxczxc-pool-0-3
LAST SEEN   TYPE      REASON             OBJECT                   MESSAGE
4m41s       Warning   FailedScheduling   pod/asdzxczxc-pool-0-3   0/5 nodes are available: 1 node(s) had taint {node-role.kubernetes.io/master: }, that the pod didn't tolerate, 1 node(s) were unschedulable, 3 node(s) had volume node affinity conflict.

Screen Shot 2021-06-14 at 10 12 24 AM

The problem is also present in pod listing, where the age of pods is not aligned with theUI

➜ k get pods -n ns-1
NAME                                     READY   STATUS    RESTARTS   AGE
asdasd-console-5c5f969756-ghd98          1/1     Running   0          12d
asdasd-log-0                             1/1     Running   0          12d
asdasd-log-search-api-657dffbcb8-9ln74   1/1     Running   3          12d
asdasd-pool-0-0                          1/1     Running   0          7m3s
asdasd-pool-0-1                          1/1     Running   0          12d
asdasd-pool-0-2                          1/1     Running   0          12d
asdasd-pool-0-3                          1/1     Running   0          12d
asdasd-prometheus-0                      2/2     Running   0          12d

Screen Shot 2021-06-14 at 10 17 35 AM

swagger.yml Outdated
properties:
namespace:
type: string
lastseen:
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename this to last_seen
Change type to integer of format: int64

for i := 0; i < len(events.Items); i++ {
retval = append(retval, &models.EventListElement{
Namespace: events.Items[i].Namespace,
Lastseen: fmt.Sprintf("%d", events.Items[i].LastTimestamp.Unix()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

the .Unix() already returns int64, store the integer that way

@dvaldivia dvaldivia requested review from Alevsk and jinapurapu June 14, 2021 18:04
const [selectedServiceAccount, setSelectedServiceAccount] = useState<
string | null
>(null);
const [selectedServiceAccount, setSelectedServiceAccount] =
Copy link
Collaborator

Choose a reason for hiding this comment

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

this needs to be formatted with prettier 2.3.1

yarn.lock Outdated
@@ -0,0 +1,4 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this file

setErrorSnackMessage(err);
});
}, [tenantNamespace, tenantName, podName, setErrorSnackMessage]);
loadInfo();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please review this

@dvaldivia
Copy link
Collaborator

Screen Shot 2021-06-15 at 10 46 47 AM

I'm getting events that are not related to the pod in question

`/api/v1/namespaces/${tenantNamespace}/tenants/${tenantName}/pods/${podName}`
)
.then((res: string) => {
setLog(res);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove setLog since log is not used

setEvent(res);
})
.catch((err) => {
setErrorSnackMessage(err);
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you remove setErrorSnackMessage you don't display an error should there be one

@@ -0,0 +1,16 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove this file

setErrorSnackMessage,
}: ITenantDetailsProps) => {
const [loading, setLoading] = useState<boolean>(false);
const [log, setLog] = useState<string>("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove unused lines

{ label: "Event Type", elementKey: "event_type" },
{ label: "Reason", elementKey: "reason" },
]}
isLoading={false}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do show a loading indicator

dvaldivia
dvaldivia previously approved these changes Jun 17, 2021
Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

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

don't delete md files

dvaldivia
dvaldivia previously approved these changes Jun 17, 2021
@dvaldivia dvaldivia merged commit 09503ed into minio:master Jun 17, 2021
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