Skip to content

Commit

Permalink
resolve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
yuwenma committed Jun 4, 2024
1 parent cab5dbd commit b83547c
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 22 deletions.
2 changes: 1 addition & 1 deletion apis/refs/v1beta1/networkref.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package v1beta1

type ComputeNetworkRef struct {
/* The , when not managed by KCC. */
/* The compute network selflink of form "projects/<project>/global/networks/<network>", when not managed by KCC. */
External string `json:"external,omitempty"`
/* The `name` field of a `ComputeNetwork` resource. */
Name string `json:"name,omitempty"`
Expand Down
7 changes: 3 additions & 4 deletions experiments/compositions/composition/proto/expander.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 4 additions & 9 deletions pkg/controller/direct/cloudbuild/workerpool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ func (m *model) AdapterForObject(ctx context.Context, reader client.Reader, u *u
networkRef, err := references.ResolveComputeNetwork(ctx, reader, obj, &obj.Spec.PrivatePoolConfig.NetworkConfig.PeeredNetworkRef)
if err != nil {
return nil, err

}
networkID = networkRef.String()
obj.Spec.PrivatePoolConfig.NetworkConfig.PeeredNetworkRef.External = networkRef.String()
}

// Get CloudBuild GCP client
Expand Down Expand Up @@ -174,15 +175,12 @@ func (a *Adapter) Create(ctx context.Context, u *unstructured.Unstructured) erro
// Use verified compute network ID.
// TODO: This is hard for users to understand. We need to make the code more readabile and self-explain.
desired := a.desired.DeepCopy()
if desired.Spec.PrivatePoolConfig.NetworkConfig != nil {
desired.Spec.PrivatePoolConfig.NetworkConfig.PeeredNetworkRef.External = a.networkID
}
wp := &cloudbuildpb.WorkerPool{
Name: a.fullyQualifiedName(),
}
err := krm.Convert_WorkerPool_KRM_To_API_v1(desired, wp)
if err != nil {
return fmt.Errorf("convert workerpool spec to api: %w", err)
return fmt.Errorf("converting workerpool spec to api: %w", err)
}
req := &cloudbuildpb.CreateWorkerPoolRequest{
Parent: a.getParent(),
Expand Down Expand Up @@ -268,12 +266,9 @@ func (a *Adapter) Update(ctx context.Context, u *unstructured.Unstructured) erro
Name: a.fullyQualifiedName(),
}
desired := a.desired.DeepCopy()
if desired.Spec.PrivatePoolConfig.NetworkConfig != nil {
desired.Spec.PrivatePoolConfig.NetworkConfig.PeeredNetworkRef.External = a.networkID
}
err := krm.Convert_WorkerPool_KRM_To_API_v1(desired, wp)
if err != nil {
return fmt.Errorf("convert workerpool spec to api: %w", err)
return fmt.Errorf("converting workerpool spec to api: %w", err)
}
req := &cloudbuildpb.UpdateWorkerPoolRequest{
WorkerPool: wp,
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/direct/references/computenetworkref.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ func ResolveComputeNetwork(ctx context.Context, reader client.Reader, src client
})
if err := reader.Get(ctx, key, computenetwork); err != nil {
if apierrors.IsNotFound(err) {
return nil, fmt.Errorf("referenced computenetwork %v not found", key)
return nil, fmt.Errorf("referenced ComputeNetwork %v not found", key)
}
return nil, fmt.Errorf("error reading referenced computenetwork %v: %w", key, err)
return nil, fmt.Errorf("error reading referenced ComputeNetwork %v: %w", key, err)
}

computenetworkID, _, err := unstructured.NestedString(computenetwork.Object, "spec", "resourceID")
if err != nil {
return nil, fmt.Errorf("reading spec.resourceID from computenetwork %v: %w", key, err)
return nil, fmt.Errorf("reading spec.resourceID from ComputeNetwork %v: %w", key, err)
}
if computenetworkID == "" {
computenetworkID = computenetwork.GetName()
Expand Down
7 changes: 5 additions & 2 deletions pkg/controller/direct/references/projectref.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func ResolveProject(ctx context.Context, reader client.Reader, src client.Object
return nil, fmt.Errorf("must specify either name or external on project reference")
}

// TODO: test case for ProjectRef with non-default namespace. What shall we expect?
key := types.NamespacedName{
Namespace: ref.Namespace,
Name: ref.Name,
Expand Down Expand Up @@ -90,16 +91,18 @@ func ResolveProject(ctx context.Context, reader client.Reader, src client.Object
}, nil
}

// ResolveProjectForTFGenerated resolves the `ProjectRef` based on Terraform generated schema, where the resourceRef
// is not dedicated to "Project" Kind.
func ResolveProjectForTFGenerated(ctx context.Context, reader client.Reader, src client.Object, ref *v1alpha1.ResourceRef) (*Project, error) {
if ref == nil {
return nil, nil
}
if ref.Kind != "Project" {
return nil, fmt.Errorf("resolve project: %s|%s|%s", ref.Kind, ref.Name, ref.Name)
return nil, fmt.Errorf("resolve project: %s. expected kind to be Project, got %q", ref.Name, ref.Kind)
}
newRef := &v1beta1.ProjectRef{
Name: ref.Name,
Namespace: ref.Name,
Namespace: ref.Namespace,
External: ref.External,
}
return ResolveProject(ctx, reader, src, newRef)
Expand Down
1 change: 0 additions & 1 deletion pkg/k8s/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ var (
KCCVersionLabel = FormatAnnotation("version")
ScopedNamespaceLabel = FormatAnnotation("scoped-namespace")
DCL2CRDLabel = FormatAnnotation("dcl2crd")
DirectCRDLabel = FormatAnnotation("directcrd")
KCCStabilityLabel = FormatAnnotation("stability-level")

MutableButUnreadableFieldsAnnotation = FormatAnnotation("mutable-but-unreadable-fields")
Expand Down

0 comments on commit b83547c

Please sign in to comment.