Change k8s ServiceTypes to be changeable by withsmilo · Pull Request #667 · ucbrise/clipper

withsmilo · 2019-04-04T04:41:12Z

When creating a KubernetesManager object, you could define own ServiceTypes. For example,

cm = KubernetesContainerManager(
         ....,
         service_types={
             'redis': 'NodePort',
             'management': 'LoadBalancer',
             'query': 'LoadBalancer',
             'query-rpc': 'ClusterIP',
             'metric': 'LoadBalancer'})

related issue : Kubernetes model deployment permissions issue #597

When creating a KubernetesManager object, you could define own ServiceTypes. For example, cm = KubernetesContainerManager( ...., service_types={ 'redis': 'NodePort', 'management': 'LoadBalancer', 'query': 'LoadBalancer', 'query-rpc': 'ClusterIP', 'metric': 'LoadBalancer'})

AmplabJenkins · 2019-04-04T05:01:43Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1885/
Test FAILed.

AmplabJenkins · 2019-04-04T05:10:47Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1886/
Test FAILed.

AmplabJenkins · 2019-04-04T05:26:19Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1888/
Test FAILed.

simon-mo · 2019-04-04T05:43:48Z

@withsmilo Thanks for this patch. However, I think we also need to patch how connect() works because it currently assumes NodePort like line:

clipper/clipper_admin/clipper_admin/kubernetes/kubernetes_container_manager.py

Line 350 in 227c8e4

self.clipper_management_port = p.node_port

rkooo567 · 2019-04-04T05:46:21Z

clipper_admin/clipper_admin/kubernetes/kubernetes_container_manager.py

            self.cluster_identifier = "{ns}-{cluster}".format(
                ns=self.k8s_namespace, cluster=self.cluster_name)

+        self.service_types = DEFAULT_CLIPPER_SERVICE_TYPES


Why don't we make this part as a function and add some tests?

@rkooo567 , OK. Let me refactor this part.

@rkooo567 , What tests are we needed here in additional?

I thought just a simple unit test? Like if this function raises Exception correctly when wrong input is given and etc. I could find that the current CI fails on line 200 (when you raise ClipperException), so it can be good if we can write some unit tests inside one of integration test files.

@withsmilo @rkooo567 can we add

def set_clipper_svc_types(self, svctype, svcname):
for key in self.OFFICIAL_K8S_SERVICE_TYPE:
if(svctype.lower()==key.lower()):
self.DEFAULT_CLIPPER_SERVICE_TYPES[svcname]=key

@rkooo567 @wcwang07
Sorry about late reply. I refactored my PR entirely.

withsmilo · 2019-04-04T05:50:52Z

@simon-mo , I will try to add something to need.

AmplabJenkins · 2019-04-04T06:13:50Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1887/
Test FAILed.

…ype_to_be_changeable

AmplabJenkins · 2019-05-03T16:06:47Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1923/
Test PASSed.

AmplabJenkins · 2019-05-03T16:49:10Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1924/
Test PASSed.

rkooo567 · 2019-05-03T21:56:59Z

clipper_admin/clipper_admin/kubernetes/kubernetes_container_manager.py

-                        self.clipper_management_port))

-            query_frontend_ports = self._k8s_v1.read_namespaced_service(
+            if self.service_types['management'] in [CLUSTER_IP, NODE_PORT]:


Why don't we read service_types from our Kubernetes definitions (yaml file) in the connect function? Otherwise we should always specify the correct self.service_types whenever we initiate KubernetesContainerManager.

It shouldn't matter when we initiate KubernetesContainerManager for the first time for start_clipper. But, let's say we already started clipper and just wanted to connect to the cluster using ClipperConnection(KubernetesContainerManager()).connect(). If you forget to write service_types, it will be set to be None and this part might be broken.

@rkooo567 You're right. Thank you for your kind advice. I will fix it as soon as possible.

@rkooo567 : According to your advice, I added a new mechanism to read/write service_types from/to a yaml file. You can connect to the executing Clipper cluster easily with service_types(None), which will be loaded automatically from a yaml file saved when we started it. If you call 'stop_all()', the yaml file will be removed.

Thank you! Everything looks good to me :)

AmplabJenkins · 2019-05-04T15:51:47Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1925/
Test PASSed.

AmplabJenkins · 2019-05-04T16:04:21Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1926/
Test FAILed.

AmplabJenkins · 2019-05-04T16:54:58Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1927/
Test PASSed.

AmplabJenkins · 2019-05-05T14:07:03Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1929/
Test PASSed.

AmplabJenkins · 2019-05-05T14:26:27Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1928/
Test PASSed.

withsmilo · 2019-05-05T14:34:46Z

@simon-mo : Please review this PR. Can we use 'LoadBalancer' service type with Travis CI?

AmplabJenkins · 2019-05-07T20:56:35Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1933/
Test PASSed.

rkooo567 · 2019-05-12T22:25:35Z

This PR looks good to me. But since it is not a simple PR, I will wait for @simon-mo's approval.

simon-mo

lgtm

AmplabJenkins · 2019-05-14T19:38:35Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1961/
Test PASSed.

withsmilo self-assigned this Apr 4, 2019

withsmilo requested a review from simon-mo April 4, 2019 04:41

Sungjun, Kim added 3 commits April 4, 2019 13:52

Fix error string

42434d5

Fix wrong set operation

e76a4f7

Change string format

0cec7da

rkooo567 reviewed Apr 4, 2019

View reviewed changes

withsmilo added the status: needs revision label Apr 5, 2019

Sungjun, Kim added 5 commits May 3, 2019 22:57

Funtionize the codes to determine service_types and add test cases.

4b80f19

Merge remote-tracking branch 'official/develop' into modify_service_t…

913b0e1

…ype_to_be_changeable

Fix bugs

c09e3fc

Support service_types(LOAD_BALANCER) on connect()

a291385

Fix a bug

15d3d37

rkooo567 reviewed May 3, 2019

View reviewed changes

Sungjun, Kim added 2 commits May 5, 2019 00:17

Write/load service_types to/from a yaml file

5bee8fa

Fix a bug

f469315

Fix bug

d63b90f

withsmilo added status: needs review and removed status: needs revision labels May 4, 2019

Sungjun, Kim added 4 commits May 5, 2019 21:48

Support multi-tenancy

f619198

Move the related test case to 'kubernetes_integration_test'

b9eb53a

Use 'cluster_identifier' instead of 'cluster_name'

9763a70

Revert clipper_admin_tests

5367e7a

withsmilo added the type: enhancement label May 5, 2019

withsmilo requested a review from RehanSD May 5, 2019 15:10

Merge branch 'develop' into modify_service_type_to_be_changeable

5f4efe1

simon-mo approved these changes May 14, 2019

View reviewed changes

Merge branch 'develop' into modify_service_type_to_be_changeable

a3ef00d

simon-mo merged commit 1bf0aa8 into ucbrise:develop May 14, 2019

withsmilo added status: accepted and removed status: needs review labels May 14, 2019

withsmilo deleted the modify_service_type_to_be_changeable branch May 14, 2019 22:14

rkooo567 mentioned this pull request May 15, 2019

[Metrics] Kubernetes Prometheus metrics address #687

Closed

Conversation

withsmilo commented Apr 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented Apr 4, 2019

Uh oh!

AmplabJenkins commented Apr 4, 2019

Uh oh!

AmplabJenkins commented Apr 4, 2019

Uh oh!

simon-mo commented Apr 4, 2019

Uh oh!

rkooo567 Apr 4, 2019

Choose a reason for hiding this comment

Uh oh!

withsmilo Apr 4, 2019

Choose a reason for hiding this comment

Uh oh!

withsmilo Apr 4, 2019

Choose a reason for hiding this comment

Uh oh!

rkooo567 Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wcwang07 Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

withsmilo May 3, 2019

Choose a reason for hiding this comment

Uh oh!

withsmilo commented Apr 4, 2019

Uh oh!

AmplabJenkins commented Apr 4, 2019

Uh oh!

AmplabJenkins commented May 3, 2019

Uh oh!

AmplabJenkins commented May 3, 2019

Uh oh!

rkooo567 May 3, 2019

Choose a reason for hiding this comment

Uh oh!

withsmilo May 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

withsmilo May 4, 2019

Choose a reason for hiding this comment

Uh oh!

rkooo567 May 5, 2019

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented May 4, 2019

Uh oh!

AmplabJenkins commented May 4, 2019

Uh oh!

AmplabJenkins commented May 4, 2019

Uh oh!

AmplabJenkins commented May 5, 2019

Uh oh!

AmplabJenkins commented May 5, 2019

Uh oh!

withsmilo commented May 5, 2019

Uh oh!

AmplabJenkins commented May 7, 2019

Uh oh!

rkooo567 commented May 12, 2019

Uh oh!

simon-mo left a comment

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented May 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

withsmilo commented Apr 4, 2019 •

edited

Loading

rkooo567 Apr 5, 2019 •

edited

Loading

wcwang07 Apr 5, 2019 •

edited

Loading

withsmilo May 3, 2019 •

edited

Loading