Skip to content

Display Kubernetes cluster name instead of uuid#4411

Merged
DaanHoogland merged 2 commits into
apache:masterfrom
ravening:k8s-error-message
Oct 21, 2020
Merged

Display Kubernetes cluster name instead of uuid#4411
DaanHoogland merged 2 commits into
apache:masterfrom
ravening:k8s-error-message

Conversation

@ravening

Copy link
Copy Markdown
Member

Description

When tried to downalod kubernetes cluster config,
it throws a pop up with uuid of kubernetes cluster
instead of its name. Displaying uuid isnt useful
even for domain admins.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

Screenshot 2020-10-16 at 12 02 14

How Has This Been Tested?

When tried to downalod kubernetes cluster config,
it throws a pop up with uuid of kubernetes cluster
instead of its name. Displaying uuid isnt useful
even for domain admins.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

makes sense

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2198

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@yadvr yadvr added this to the 4.15.0.0 milestone Oct 16, 2020

@davidjumani davidjumani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Pearl1594 Pearl1594 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good

@yadvr yadvr closed this Oct 19, 2020
@yadvr yadvr reopened this Oct 19, 2020
@DaanHoogland

Copy link
Copy Markdown
Contributor

I'm not going to trust that last test run
@blueorangutan test

@shwstppr shwstppr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes are okay but CKS code has cluster UUID showing at numerous other places in exception messages. Should we change them all for consistency? @ravening

@ravening

Copy link
Copy Markdown
Member Author

Changes are okay but CKS code has cluster UUID showing at numerous other places in exception messages. Should we change them all for consistency? @ravening

@shwstppr it's better to change right? Showing uuid to user or admin is not helpful.

I'm stuck in creating cluster and haven't proceeded further. So don't know where else to change. Only way is to scan the code

@shwstppr

Copy link
Copy Markdown
Contributor

@ravening still facing #4146 ?

@ravening

Copy link
Copy Markdown
Member Author

@ravening still facing #4146 ?

Yes

@apache apache deleted a comment from yadvr Oct 20, 2020
@apache apache deleted a comment from blueorangutan Oct 20, 2020
@apache apache deleted a comment from blueorangutan Oct 20, 2020
@apache apache deleted a comment from blueorangutan Oct 20, 2020
@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2212

@DaanHoogland

Copy link
Copy Markdown
Contributor

@shwstppr all concerns addressed?

@GabrielBrascher GabrielBrascher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code LGTM

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@ravening

Copy link
Copy Markdown
Member Author

@shwstppr all concerns addressed?

@shwstppr made changes in few more places where I could reproduce the issue. Didnt do in remaining places

@shwstppr

Copy link
Copy Markdown
Contributor

LGTM on current changes. There are few more places where we will need change. We can have a new PR later?

@davidjumani

davidjumani commented Oct 20, 2020

Copy link
Copy Markdown
Contributor

@ravening @shwstppr I can raise a PR on the remaining places since I'm trying to enhance Kubernetes cluster at the moment. Let me know if that works

@ravening

Copy link
Copy Markdown
Member Author

@ravening @shwstppr I can raise a PR on the remaining places since I'm trying to enhance Kubernetes cluster at the moment. Let me know if that works

@davidjumani sounds good to me

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-3019)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46145 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4411-t3019-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 82 look OK, 3 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_03_deploy_and_upgrade_kubernetes_cluster Failure 237.08 test_kubernetes_clusters.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL Failure 310.39 test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers Failure 305.45 test_vpc_redundant.py
test_hostha_kvm_host_fencing Error 169.60 test_hostha_kvm.py

@DaanHoogland

Copy link
Copy Markdown
Contributor

@shwstppr the kubernetes upgrade test is the only one related to this.

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/marvin/lib/decoratorGenerators.py", line 30, in test_wrapper
    return test(self, *args, **kwargs)
  File "/marvin/tests/smoke/test_kubernetes_clusters.py", line 390, in test_03_deploy_and_upgrade_kubernetes_cluster
    self.fail("Failed to upgrade Kubernetes cluster due to: %s" % e)
AssertionError: Failed to upgrade Kubernetes cluster due to: Job failed: {jobprocstatus : 0, created : u'2020-10-20T16:35:13+0000', completed : u'2020-10-20T16:37:17+0000', cmd : u'org.apache.cloudstack.api.command.user.kubernetes.cluster.UpgradeKubernetesClusterCmd', userid : u'a7f44f67-12d8-11eb-a1eb-1e0062013a36', jobstatus : 2, jobid : u'a1a0ffa1-dd5e-4eef-b572-258e421aa556', jobresultcode : 530, jobresulttype : u'object', jobresult : {errorcode : 530, errortext : u'Failed to upgrade Kubernetes cluster ID: 1c501beb-102e-49e2-8cb6-16b6056b1f96, unable to upgrade Kubernetes node on VM ID: e911af24-3b44-40a0-918e-ac5c5abd31b4'}, accountid : u'a7f2a712-12d8-11eb-a1eb-1e0062013a36'}

can you check?

@shwstppr

Copy link
Copy Markdown
Contributor

@DaanHoogland seems like some network issue in env during cluster upgrade. Failed from kubeadm itself. Doesn't seem related to PR changes,

121340 2020-10-20 16:37:14,144 ERROR [c.c.u.s.SshHelper] (API-Job-Executor-5:ctx-928e799d job-779 ctx-7bc68c76) (logid:a1a0ffa1) SSH execution of command sudo ./upgrade-kuberntes13470214930726030527.sh 1       .16.0 false false false has an error status code in return. Result output: Installing binaries from /mnt/k8sdisk/
121341 Loaded image: k8s.gcr.io/coredns:1.6.2                                             
121342 Loaded image: k8s.gcr.io/etcd:3.3.15-0                                             
121343 Loaded image: k8s.gcr.io/kube-apiserver:v1.16.0                                    
121344 Loaded image: k8s.gcr.io/kube-controller-manager:v1.16.0                           
121345 Loaded image: k8s.gcr.io/kube-proxy:v1.16.0                                        
121346 Loaded image: k8s.gcr.io/kube-scheduler:v1.16.0                                    
121347 Loaded image: k8s.gcr.io/pause:3.1                                                 
121348 [upgrade] Reading configuration from the cluster...                                
121349 [upgrade] FYI: You can look at this config file with 'kubectl -n kube-system get cm kubeadm-config -oyaml'
121350 unable to fetch the kubeadm-config ConfigMap: failed to get config map: Get https://10.1.1.234:6443/api/v1/namespaces/kube-system/configmaps/kubeadm-config: dial tcp 10.1.1.234:6443: connect: conn       ection refused
121351 To see the stack trace of this error execute with --v=5 or higher                  
121352                                                                                    
121353 2020-10-20 16:37:14,145 ERROR [c.c.k.c.a.KubernetesClusterActionWorker] (API-Job-Executor-5:ctx-928e799d job-779 ctx-7bc68c76) (logid:a1a0ffa1) Failed to upgrade Kubernetes cluster ID: 1c501beb-10       2e-49e2-8cb6-16b6056b1f96, unable to upgrade Kubernetes node on VM ID: e911af24-3b44-40a0-918e-ac5c5abd31b4

If needed we can have another test run or test upgrade manually.

@DaanHoogland

Copy link
Copy Markdown
Contributor

so we have an lgtm from you? (can you reflect that in your review status?

@shwstppr shwstppr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cluster upgrade test failure doesn't seem related. LGTM

@DaanHoogland DaanHoogland merged commit a5d4bd5 into apache:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants