Skip to content

Commit 75f9d84

Browse files
yue9944882yue9944882
committed
addressing review comments
1 parent c0ea154 commit 75f9d84

File tree

6 files changed

+63
-89
lines changed

6 files changed

+63
-89
lines changed

‎extended/src/main/java/io/kubernetes/client/extended/kubectl/Kubectl.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,17 @@ public static interface Executable<OUTPUT> {
145145
OUTPUT execute() throws KubectlException;
146146
}
147147

148-
abstract static class ApiClientBuilder<T extends ApiClientBuilder> {
148+
abstract static class NamespacedApiClientBuilder<T extends NamespacedApiClientBuilder>
149+
extends ApiClientBuilder<T> {
150+
String namespace;
151+
152+
public T namespace(String namespace) {
153+
this.namespace = namespace;
154+
return (T) this;
155+
}
156+
}
149157

158+
abstract static class ApiClientBuilder<T extends ApiClientBuilder> {
150159
ApiClient apiClient = Configuration.getDefaultApiClient();
151160

152161
public T apiClient(ApiClient apiClient) {
@@ -157,9 +166,8 @@ public T apiClient(ApiClient apiClient) {
157166

158167
abstract static class ResourceBuilder<
159168
ApiType extends KubernetesObject, T extends ResourceBuilder<ApiType, T>>
160-
extends ApiClientBuilder<T> {
169+
extends NamespacedApiClientBuilder<T> {
161170
final Class<ApiType> apiTypeClass;
162-
String namespace;
163171
String name;
164172
String apiGroup;
165173
String apiVersion;
@@ -174,11 +182,6 @@ public T name(String name) {
174182
return (T) this;
175183
}
176184

177-
public T namespace(String namespace) {
178-
this.namespace = namespace;
179-
return (T) this;
180-
}
181-
182185
public T apiGroup(String apiGroup) {
183186
this.apiGroup = apiGroup;
184187
return (T) this;

‎extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlCreate.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
import com.google.common.base.Strings;
1616
import io.kubernetes.client.Discovery;
17+
import io.kubernetes.client.apimachinery.GroupVersionKind;
1718
import io.kubernetes.client.common.KubernetesListObject;
1819
import io.kubernetes.client.common.KubernetesObject;
1920
import io.kubernetes.client.extended.kubectl.exception.KubectlException;
21+
import io.kubernetes.client.openapi.ApiException;
2022
import io.kubernetes.client.util.ModelMapper;
2123
import io.kubernetes.client.util.Namespaces;
2224
import io.kubernetes.client.util.generic.GenericKubernetesApi;
@@ -25,14 +27,14 @@
2527
import java.util.Optional;
2628
import java.util.Set;
2729

28-
public class KubectlCreate extends Kubectl.ApiClientBuilder<KubectlCreate>
30+
public class KubectlCreate extends Kubectl.NamespacedApiClientBuilder<KubectlCreate>
2931
implements Kubectl.Executable<KubernetesObject> {
3032

3133
KubectlCreate() {}
3234

3335
private KubernetesObject targetObj;
3436

35-
public KubectlCreate load(KubernetesObject obj) {
37+
public KubectlCreate resource(KubernetesObject obj) {
3638
this.targetObj = obj;
3739
return this;
3840
}
@@ -41,11 +43,11 @@ public KubectlCreate load(KubernetesObject obj) {
4143
public KubernetesObject execute() throws KubectlException {
4244
Discovery discovery = new Discovery(apiClient);
4345
try {
46+
// TODO(yue9944882): caches api discovery in memory
4447
Set<Discovery.APIResource> apiResources = discovery.findAll();
4548
ModelMapper.refresh(apiResources);
4649

47-
ModelMapper.GroupVersionKind gvk =
48-
ModelMapper.getGroupVersionKindByClass(targetObj.getClass());
50+
GroupVersionKind gvk = ModelMapper.getGroupVersionKindByClass(targetObj.getClass());
4951
Optional<Discovery.APIResource> apiResource =
5052
apiResources.stream()
5153
.filter(r -> r.getKind().equals(gvk.getKind()) && r.getGroup().equals(gvk.getGroup()))
@@ -63,20 +65,23 @@ public KubernetesObject execute() throws KubectlException {
6365
apiResource.get().getResourcePlural(),
6466
apiClient);
6567
if (apiResource.get().getNamespaced()) {
66-
String namespace =
67-
Strings.isNullOrEmpty(targetObj.getMetadata().getNamespace())
68-
? Namespaces.NAMESPACE_DEFAULT
69-
: targetObj.getMetadata().getNamespace();
68+
String targetNamespace =
69+
namespace != null
70+
? namespace
71+
: Strings.isNullOrEmpty(targetObj.getMetadata().getNamespace())
72+
? Namespaces.NAMESPACE_DEFAULT
73+
: targetObj.getMetadata().getNamespace();
74+
7075
KubernetesApiResponse<KubernetesObject> response =
71-
api.create(namespace, targetObj, new CreateOptions());
76+
api.create(targetNamespace, targetObj, new CreateOptions());
7277
return response.getObject();
7378
} else {
7479
KubernetesApiResponse<KubernetesObject> response =
7580
api.create(targetObj, new CreateOptions());
7681
return response.getObject();
7782
}
78-
} catch (Throwable t) {
79-
throw new KubectlException(t);
83+
} catch (ApiException e) {
84+
throw new KubectlException(e);
8085
}
8186
}
8287
}

‎extended/src/test/java/io/kubernetes/client/extended/kubectl/KubectlCreateTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void testCreateConfigMap() throws KubectlException, IOException {
8080
(V1ConfigMap)
8181
Kubectl.create()
8282
.apiClient(apiClient)
83-
.load(
83+
.resource(
8484
new V1ConfigMap()
8585
.metadata(new V1ObjectMeta().namespace("foo").name("bar"))
8686
.data(

‎util/src/main/java/io/kubernetes/client/apimachinery/GroupVersionKind.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
package io.kubernetes.client.apimachinery;
1414

15+
import com.google.common.base.Objects;
16+
1517
public class GroupVersionKind {
1618

1719
private final String group;
@@ -35,4 +37,19 @@ public String getVersion() {
3537
public String getKind() {
3638
return kind;
3739
}
40+
41+
@Override
42+
public boolean equals(Object o) {
43+
if (this == o) return true;
44+
if (o == null || getClass() != o.getClass()) return false;
45+
GroupVersionKind that = (GroupVersionKind) o;
46+
return Objects.equal(group, that.group)
47+
&& Objects.equal(version, that.version)
48+
&& Objects.equal(kind, that.kind);
49+
}
50+
51+
@Override
52+
public int hashCode() {
53+
return Objects.hashCode(group, version, kind);
54+
}
3855
}

‎util/src/main/java/io/kubernetes/client/util/ModelMapper.java

Lines changed: 6 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
*/
1313
package io.kubernetes.client.util;
1414

15-
import com.google.common.base.Objects;
1615
import com.google.common.collect.ImmutableMap;
1716
import com.google.common.reflect.ClassPath;
1817
import io.kubernetes.client.Discovery;
18+
import io.kubernetes.client.apimachinery.GroupVersionKind;
1919
import io.kubernetes.client.common.KubernetesListObject;
2020
import io.kubernetes.client.common.KubernetesObject;
2121
import io.kubernetes.client.openapi.ApiException;
@@ -143,6 +143,8 @@ public static GroupVersionKind getGroupVersionKindByClass(Class<?> clazz) {
143143
* Refreshes the model mapping by syncing up w/the api discovery info from the kubernetes
144144
* apiserver.
145145
*
146+
* <p>Note: if model mappings can be incomplete if this method is never called.
147+
*
146148
* @param discovery the discovery
147149
* @throws ApiException the api exception
148150
*/
@@ -154,10 +156,12 @@ public static void refresh(Discovery discovery) throws ApiException {
154156
* Refreshes the model mapping by syncing up w/the api discovery info from the kubernetes
155157
* apiserver.
156158
*
159+
* <p>Note: if model mappings can be incomplete if this method is never called.
160+
*
157161
* @param apiResources the api resources
158162
* @throws ApiException the api exception
159163
*/
160-
public static void refresh(Set<Discovery.APIResource> apiResources) throws ApiException {
164+
public static void refresh(Set<Discovery.APIResource> apiResources) {
161165
for (Discovery.APIResource apiResource : apiResources) {
162166
for (String version : apiResource.getVersions()) {
163167
Class<?> clazz = getApiTypeClass(apiResource.getGroup(), version, apiResource.getKind());
@@ -243,59 +247,4 @@ private static Pair<String, String> getApiVersion(String name) {
243247
.findFirst()
244248
.orElse(new MutablePair(null, name));
245249
}
246-
247-
public static class GroupVersionKind {
248-
249-
public GroupVersionKind(String group, String version, String kind) {
250-
this.group = group;
251-
this.version = version;
252-
this.kind = kind;
253-
}
254-
255-
private final String group;
256-
private final String version;
257-
private final String kind;
258-
259-
public String getGroup() {
260-
return group;
261-
}
262-
263-
public String getVersion() {
264-
return version;
265-
}
266-
267-
public String getKind() {
268-
return kind;
269-
}
270-
271-
@Override
272-
public boolean equals(Object o) {
273-
if (this == o) return true;
274-
if (o == null || getClass() != o.getClass()) return false;
275-
GroupVersionKind that = (GroupVersionKind) o;
276-
return Objects.equal(group, that.group)
277-
&& Objects.equal(version, that.version)
278-
&& Objects.equal(kind, that.kind);
279-
}
280-
281-
@Override
282-
public int hashCode() {
283-
return Objects.hashCode(group, version, kind);
284-
}
285-
286-
@Override
287-
public String toString() {
288-
return "GroupVersionKind{"
289-
+ "group='"
290-
+ group
291-
+ '\''
292-
+ ", version='"
293-
+ version
294-
+ '\''
295-
+ ", kind='"
296-
+ kind
297-
+ '\''
298-
+ '}';
299-
}
300-
}
301250
}

‎util/src/test/java/io/kubernetes/client/util/ModelMapperTest.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212
*/
1313
package io.kubernetes.client.util;
1414

15-
import static org.junit.Assert.*;
15+
import static org.junit.Assert.assertEquals;
16+
import static org.junit.Assert.assertNull;
17+
import static org.junit.Assert.assertTrue;
1618

1719
import io.kubernetes.client.Discovery;
20+
import io.kubernetes.client.apimachinery.GroupVersionKind;
1821
import io.kubernetes.client.openapi.ApiException;
1922
import io.kubernetes.client.openapi.models.V1Deployment;
2023
import io.kubernetes.client.openapi.models.V1Pod;
@@ -28,8 +31,7 @@ public class ModelMapperTest {
2831

2932
@Test
3033
public void testPrebuiltModelMapping() {
31-
32-
Map<ModelMapper.GroupVersionKind, Class<?>> maps = ModelMapper.getAllKnownClasses();
34+
Map<GroupVersionKind, Class<?>> maps = ModelMapper.getAllKnownClasses();
3335
assertTrue(maps.size() > 0);
3436

3537
assertEquals(V1Pod.class, ModelMapper.getApiTypeClass("", "v1", "Pod"));
@@ -69,17 +71,17 @@ public void testAddingModel() {
6971
@Test
7072
public void testGetClassByKind() {
7173
assertEquals(
72-
new ModelMapper.GroupVersionKind(null, "v1", "Pod"),
74+
new GroupVersionKind(null, "v1", "Pod"),
7375
ModelMapper.getGroupVersionKindByClass(V1Pod.class));
7476

7577
assertEquals(
76-
new ModelMapper.GroupVersionKind(null, "v1", "Pod"),
78+
new GroupVersionKind(null, "v1", "Pod"),
7779
ModelMapper.getGroupVersionKindByClass(V1Pod.class));
7880
assertEquals(
79-
new ModelMapper.GroupVersionKind(null, "v1", "Deployment"),
81+
new GroupVersionKind(null, "v1", "Deployment"),
8082
ModelMapper.getGroupVersionKindByClass(V1Deployment.class));
8183
assertEquals(
82-
new ModelMapper.GroupVersionKind(null, "v1beta1", "CustomResourceDefinition"),
84+
new GroupVersionKind(null, "v1beta1", "CustomResourceDefinition"),
8385
ModelMapper.getGroupVersionKindByClass(V1beta1CustomResourceDefinition.class));
8486
}
8587

@@ -91,14 +93,12 @@ public void test() throws IOException, ApiException {
9193
ModelMapper.refresh(discovery);
9294

9395
assertEquals(
94-
new ModelMapper.GroupVersionKind("", "v1", "Pod"),
95-
ModelMapper.getGroupVersionKindByClass(V1Pod.class));
96+
new GroupVersionKind("", "v1", "Pod"), ModelMapper.getGroupVersionKindByClass(V1Pod.class));
9697
assertEquals(
97-
new ModelMapper.GroupVersionKind("apps", "v1", "Deployment"),
98+
new GroupVersionKind("apps", "v1", "Deployment"),
9899
ModelMapper.getGroupVersionKindByClass(V1Deployment.class));
99100
assertEquals(
100-
new ModelMapper.GroupVersionKind(
101-
"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition"),
101+
new GroupVersionKind("apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition"),
102102
ModelMapper.getGroupVersionKindByClass(V1beta1CustomResourceDefinition.class));
103103
}
104104
}

0 commit comments

Comments
 (0)