-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 Adding controller runtime create #9736
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -104,7 +104,7 @@ func ApplyAutoscalerToWorkloadCluster(ctx context.Context, input ApplyAutoscaler | |||||
}, | ||||||
}) | ||||||
Expect(err).ToNot(HaveOccurred(), "failed to parse %s", workloadYamlTemplate) | ||||||
Expect(input.WorkloadClusterProxy.Apply(ctx, workloadYaml)).To(Succeed(), "failed to apply %s", workloadYamlTemplate) | ||||||
Expect(input.WorkloadClusterProxy.Create(ctx, workloadYaml)).To(Succeed(), "failed to apply %s", workloadYamlTemplate) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
By("Wait for the autoscaler deployment and collect logs") | ||||||
deployment := &appsv1.Deployment{ | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,9 @@ import ( | |
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
corev1 "k8s.io/api/core/v1" | ||
apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
"k8s.io/apimachinery/pkg/runtime" | ||
kerrors "k8s.io/apimachinery/pkg/util/errors" | ||
"k8s.io/apimachinery/pkg/util/wait" | ||
"k8s.io/client-go/kubernetes" | ||
"k8s.io/client-go/rest" | ||
|
@@ -45,6 +47,7 @@ import ( | |
"sigs.k8s.io/cluster-api/test/framework/exec" | ||
"sigs.k8s.io/cluster-api/test/framework/internal/log" | ||
"sigs.k8s.io/cluster-api/test/infrastructure/container" | ||
"sigs.k8s.io/cluster-api/util/yaml" | ||
) | ||
|
||
const ( | ||
|
@@ -90,6 +93,9 @@ type ClusterProxy interface { | |
// Apply to apply YAML to the Kubernetes cluster, `kubectl apply`. | ||
Apply(ctx context.Context, resources []byte, args ...string) error | ||
|
||
// Create creates using the controller-runtime client. | ||
Create(ctx context.Context, resources []byte) error | ||
|
||
// GetWorkloadCluster returns a proxy to a workload cluster defined in the Kubernetes cluster. | ||
GetWorkloadCluster(ctx context.Context, namespace, name string, options ...Option) ClusterProxy | ||
|
||
|
@@ -253,6 +259,27 @@ func (p *clusterProxy) Apply(ctx context.Context, resources []byte, args ...stri | |
return exec.KubectlApply(ctx, p.kubeconfigPath, resources, args...) | ||
} | ||
|
||
// Create creates using the controller-runtime client. | ||
func (p *clusterProxy) Create(ctx context.Context, resources []byte) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a create func? Why not use GetClient().Create ? I'm a bit concerned about silently ignoring IsAlreadyExists errors on this level (could lead to all sorts of surprises for the caller of this func if its YAML is not deployed) |
||
Expect(ctx).NotTo(BeNil(), "ctx is required for Create") | ||
Expect(resources).NotTo(BeNil(), "resources is required for Create") | ||
|
||
var retErrs []error | ||
objs, err := yaml.ToUnstructured(resources) | ||
if err != nil { | ||
return err | ||
} | ||
for o := range objs { | ||
if err := p.GetClient().Create(ctx, &objs[o]); err != nil { | ||
if apierrors.IsAlreadyExists(err) { | ||
continue | ||
} | ||
Comment on lines
+274
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What problem are we trying to solve here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See: #9736 (comment) |
||
retErrs = append(retErrs, err) | ||
} | ||
} | ||
return kerrors.NewAggregate(retErrs) | ||
} | ||
|
||
func (p *clusterProxy) GetRESTConfig() *rest.Config { | ||
config, err := clientcmd.LoadFromFile(p.kubeconfigPath) | ||
Expect(err).ToNot(HaveOccurred(), "Failed to load Kubeconfig file from %q", p.kubeconfigPath) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.