Skip to content

Conversation

@enxebre
Copy link
Member

@enxebre enxebre commented Aug 30, 2018

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 30, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2018
@enxebre
Copy link
Member Author

enxebre commented Aug 30, 2018

Superseeds #32
Let's get this merged, then as a follow up include support for libvirt
cc @bison @ingvagabund


glog.Info("Trying to deploy MachineSet object")
_, err = v1alphaClient.MachineSets("test").Create(machineSet)
_, err = v1alphaClient.MachineSets("default").Create(machineSet)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default still better than test. Though, is this the final namespace? I would rather make the namespace configurable. Either via configmap or via mao --namespace or similar option.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. Yes, there's separate tickets for addressing dedicated namespaces and service accounts.

@enxebre enxebre mentioned this pull request Aug 30, 2018
- filters:
- name: "tag:Name"
values:
- {{.ClusterName}}_worker_sg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably fine for now, but just a thought: If we're going to select security groups this way, we may want to also filter by cluster ID if that's available.

- {{.ClusterName}}-worker-{{.AvailabilityZone}}
publicIp: true
iamInstanceProfile:
id: {{.ClusterName}}-master-profile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be -worker-profile?

@openshift-merge-robot openshift-merge-robot merged commit 04253cb into openshift:master Aug 30, 2018
wking added a commit to wking/machine-api-operator that referenced this pull request Oct 4, 2018
We've had this since 04253cb (add support for openshift actuator,
2018-08-29, openshift#34), but we don't need direct access to the workers.
Folks who do need to SSH into them, or whatever, should be able to
access them via the masters.
ingvagabund added a commit to ingvagabund/machine-api-operator that referenced this pull request Jul 11, 2019
Requeue machine every 20s until running.
germanparente pushed a commit to germanparente/machine-api-operator that referenced this pull request Sep 23, 2025
germanparente pushed a commit to germanparente/machine-api-operator that referenced this pull request Sep 23, 2025
OCPCLOUD-1804: Port to ginkgo v2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants