Skip to content

Commit 76f7cef

Browse files
committed
comments addressed
1 parent 4cb8cc5 commit 76f7cef

5 files changed

Lines changed: 59 additions & 182 deletions

File tree

packages/core/src/awsService/sagemaker/detached-server/kubectlClientStub.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export interface HyperpodCluster {
2424
clusterName: string
2525
clusterArn: string
2626
status: string
27+
eksClusterName?: string
28+
eksClusterArn?: string
2729
regionCode: string
2830
}
2931

@@ -34,24 +36,30 @@ export interface WorkspaceConnectionResult {
3436
sessionId: string
3537
}
3638

37-
/** Refresh the token 1 minute before it expires. */
39+
/** Buffer time (ms) before token expiry to trigger a proactive refresh, avoiding mid-request expirations. */
3840
const tokenRefreshBufferMs = 60_000
3941

42+
export interface EksClusterInfo {
43+
name?: string
44+
endpoint?: string
45+
certificateAuthority?: { data?: string }
46+
}
47+
4048
export class KubectlClient {
4149
private kubeConfig: k8s.KubeConfig
42-
private k8sApi!: k8s.CustomObjectsApi
50+
private k8sApi: k8s.CustomObjectsApi | undefined
4351
private tokenExpiry: number = 0
4452

45-
private constructor(
46-
private readonly eksCluster: any,
53+
protected constructor(
54+
private readonly eksCluster: EksClusterInfo,
4755
private readonly hyperpodCluster: HyperpodCluster,
4856
private readonly credentials: AwsCredentialIdentity | Provider<AwsCredentialIdentity>
4957
) {
5058
this.kubeConfig = new k8s.KubeConfig()
5159
}
5260

53-
static async create(
54-
eksCluster: any,
61+
static async createForCluster(
62+
eksCluster: EksClusterInfo,
5563
hyperpodCluster: HyperpodCluster,
5664
credentials: AwsCredentialIdentity | Provider<AwsCredentialIdentity>
5765
): Promise<KubectlClient> {
@@ -60,10 +68,17 @@ export class KubectlClient {
6068
return client
6169
}
6270

63-
getEksCluster(): any {
71+
getEksCluster(): EksClusterInfo {
6472
return this.eksCluster
6573
}
6674

75+
protected getApi(): k8s.CustomObjectsApi {
76+
if (!this.k8sApi) {
77+
throw new Error('[Hyperpod] KubectlClient not initialized — call createForCluster()')
78+
}
79+
return this.k8sApi
80+
}
81+
6782
async createWorkspaceConnection(devSpace: HyperpodDevSpace): Promise<WorkspaceConnectionResult> {
6883
try {
6984
await this.ensureValidToken()
@@ -84,7 +99,7 @@ export class KubectlClient {
8499
},
85100
}
86101

87-
const response = await this.k8sApi.createNamespacedCustomObject(
102+
const response = await this.getApi().createNamespacedCustomObject(
88103
group,
89104
version,
90105
devSpace.namespace,
@@ -96,6 +111,8 @@ export class KubectlClient {
96111
const status = body.status
97112
const presignedUrl = status?.workspaceConnectionUrl
98113
const connectionType = status?.workspaceConnectionType
114+
// tokenValue and sessionId may not be present in all API responses.
115+
// When empty, the existing connection flow in model.ts falls back to parsing these from the presigned URL.
99116
const token = status?.tokenValue ?? ''
100117
const sessionId = status?.sessionId ?? ''
101118

@@ -107,7 +124,7 @@ export class KubectlClient {
107124
}
108125
}
109126

110-
private async initKubeConfig(): Promise<void> {
127+
protected async initKubeConfig(): Promise<void> {
111128
if (!this.eksCluster.name || !this.eksCluster.endpoint) {
112129
return
113130
}
@@ -147,13 +164,13 @@ export class KubectlClient {
147164
this.k8sApi = this.kubeConfig.makeApiClient(k8s.CustomObjectsApi)
148165
}
149166

150-
private async ensureValidToken(): Promise<void> {
167+
protected async ensureValidToken(): Promise<void> {
151168
if (Date.now() >= this.tokenExpiry - tokenRefreshBufferMs) {
152169
await this.refreshToken()
153170
}
154171
}
155172

156-
private async refreshToken(): Promise<void> {
173+
protected async refreshToken(): Promise<void> {
157174
if (!this.eksCluster.name) {
158175
return
159176
}

packages/core/src/awsService/sagemaker/explorer/sagemakerHyperpodNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export class SagemakerHyperpodNode extends AWSTreeNodeBase {
100100
}
101101
return creds as AwsCredentialIdentity
102102
}
103-
kcClient = await KubectlClient.create(eksCluster, cluster, credentialsProvider)
103+
kcClient = await KubectlClient.createForCluster(eksCluster, cluster, credentialsProvider)
104104
this.kubectlClients.set(cluster.clusterName, kcClient)
105105
}
106106
const spacesPerCluster = await kcClient.getSpacesForCluster(eksCluster)

packages/core/src/awsService/sagemaker/hyperpodCommands.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ export async function connectToHyperPodDevSpace(node: SagemakerDevSpaceNode): Pr
6565
// Call createWorkspaceConnection to get presigned URL with credentials
6666
const workspaceConnection = await kubectlClient.createWorkspaceConnection(node.devSpace)
6767
const connectionUrl = workspaceConnection.url
68-
const connectionToken = workspaceConnection.token
6968

7069
const remoteEnv = await prepareDevEnvConnection({
7170
spaceArn: '',
@@ -81,7 +80,6 @@ export async function connectToHyperPodDevSpace(node: SagemakerDevSpaceNode): Pr
8180
eksEndpoint: eksCluster?.endpoint,
8281
eksCertAuthData: eksCluster?.certificateAuthority?.data,
8382
wsUrl: connectionUrl,
84-
token: connectionToken,
8583
})
8684

8785
await startVscodeRemote(

packages/core/src/awsService/sagemaker/model.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,17 +181,12 @@ export async function prepareDevEnvConnection(opts: DevEnvConnectionOptions) {
181181
const sshPrefix = getSshPrefix(connectionType)
182182
let hostname: string
183183
if (connectionType === 'sm_hp') {
184-
// Always construct hostname from workspace components for a meaningful window title.
185-
// The `session` param is the SSM sessionId (e.g. "eks-sagemaker-jupyter"), not a hostname.
186-
let hpSession: string | undefined
187-
if (workspaceName && namespace && clusterName && region && accountId) {
184+
let hpSession = session
185+
if (!hpSession) {
188186
const proposedSession = `${workspaceName}_${namespace}_${clusterName}_${region}_${accountId}`
189187
hpSession = isValidSshHostname(proposedSession)
190188
? proposedSession
191-
: createValidSshSession(workspaceName, namespace, clusterName, region, accountId)
192-
}
193-
if (!hpSession) {
194-
hpSession = session || 'hyperpod'
189+
: createValidSshSession(workspaceName!, namespace!, clusterName!, region!, accountId!)
195190
}
196191
hostname = `${sshPrefix}${hpSession}`
197192
} else {

0 commit comments

Comments
 (0)