Skip to content
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

Support Roles on OpenStackDataPlaneService #1227

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
with:
# this fetches all branches. Needed because we need gh-pages branch for deploy to work
fetch-depth: 0
- uses: ruby/setup-ruby@v1.160.0
- uses: ruby/setup-ruby@v1
with:
ruby-version: '3.2'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ spec:
type: string
playbookContents:
type: string
role:
type: string
tlsCerts:
additionalProperties:
properties:
Expand Down
3 changes: 3 additions & 0 deletions apis/dataplane/v1beta1/openstackdataplaneservice_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ type OpenStackDataPlaneServiceSpec struct {
// Playbook is a path to the playbook that ansible will run on this execution
Playbook string `json:"playbook,omitempty"`

// Role is a path to the role that ansible will run on this execution
Role string `json:"role,omitempty"`

// CACerts - Secret containing the CA certificate chain
// +kubebuilder:validation:Optional
// +kubebuilder:validation:MaxLength:=253
Expand Down
27 changes: 22 additions & 5 deletions apis/dataplane/v1beta1/openstackdataplaneservice_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,31 @@ func (r *OpenStackDataPlaneService) ValidateCreate() (admission.Warnings, error)
return nil, nil
}

func (r *OpenStackDataPlaneServiceSpec) ValidateCreate() field.ErrorList {
// TODO(user): fill in your validation logic upon object creation.
func (r *OpenStackDataPlaneServiceSpec) ValidateArtifact() field.ErrorList {
if len(r.Playbook) == len(r.PlaybookContents) && len(r.Playbook) == len(r.Role) && len(r.Playbook) == 0 {
return field.ErrorList{
field.Invalid(
field.NewPath("Playbook"),
r.Playbook, "Playbook, PlaybookContents and Role cannot be empty at the same time",
),
field.Invalid(
field.NewPath("PlaybookContents"),
r.Playbook, "Playbook, PlaybookContents and Role cannot be empty at the same time",
),
field.Invalid(
field.NewPath("Role"),
r.Playbook, "Playbook, PlaybookContents and Role cannot be empty at the same time",
),
}
}

return field.ErrorList{}
}

func (r *OpenStackDataPlaneServiceSpec) ValidateCreate() field.ErrorList {
return r.ValidateArtifact()
}

func (r *OpenStackDataPlaneService) ValidateUpdate(original runtime.Object) (admission.Warnings, error) {
openstackdataplaneservicelog.Info("validate update", "name", r.Name)
errors := r.Spec.ValidateUpdate()
Expand All @@ -100,9 +119,7 @@ func (r *OpenStackDataPlaneService) ValidateUpdate(original runtime.Object) (adm
}

func (r *OpenStackDataPlaneServiceSpec) ValidateUpdate() field.ErrorList {
// TODO(user): fill in your validation logic upon object creation.

return field.ErrorList{}
return r.ValidateArtifact()
}

func (r *OpenStackDataPlaneService) ValidateDelete() (admission.Warnings, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ spec:
type: string
playbookContents:
type: string
role:
type: string
tlsCerts:
additionalProperties:
properties:
Expand Down
3 changes: 3 additions & 0 deletions pkg/dataplane/util/ansible_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ func (a *EEJob) BuildAeeJobSpec(
if len(service.Spec.Playbook) > 0 {
a.Playbook = service.Spec.Playbook
}
if len(service.Spec.Role) > 0 {
a.Role = service.Spec.Role
}

a.BackoffLimit = deployment.Spec.BackoffLimit
a.PreserveJobs = deployment.Spec.PreserveJobs
Expand Down
25 changes: 19 additions & 6 deletions pkg/dataplane/util/ansibleee.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ type EEJob struct {
PlaybookContents string `json:"playbookContents,omitempty"`
// Playbook is the playbook that ansible will run on this execution, accepts path or FQN from collection
Playbook string `json:"playbook,omitempty"`
// Role is the role that ansible will run on this execution, accepts path or FQN from collection
Role string `json:"role,omitempty"`
// Image is the container image that will execute the ansible command
Image string `json:"image,omitempty"`
// Name is the name of the execution job
Expand Down Expand Up @@ -78,12 +80,20 @@ func (a *EEJob) JobForOpenStackAnsibleEE(h *helper.Helper) (*batchv1.Job, error)

args := a.Args

playbook := a.Playbook
if len(args) == 0 {
if len(playbook) == 0 {
playbook = CustomPlaybook
artifact := a.Playbook
param := "-p"
if len(artifact) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we still saying role if provided has the highest precedence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh! I misplaced the order

if len(a.PlaybookContents) > 0 {
artifact = CustomPlaybook
} else if len(a.Role) > 0 {
artifact = a.Role
param = "-r"
} else {
return nil, fmt.Errorf("no playbook, playbookContents or role specified")
}
}
args = []string{"ansible-runner", "run", "/runner", "-p", playbook}
args = []string{"ansible-runner", "run", "/runner", param, artifact}
}

// ansible runner identifier
Expand Down Expand Up @@ -169,12 +179,15 @@ func (a *EEJob) JobForOpenStackAnsibleEE(h *helper.Helper) (*batchv1.Job, error)
}
}

if len(a.Role) > 0 {
setRunnerEnvVar(h, "RUNNER_ROLE", a.Role, "role", job, hashes)
}
if len(a.PlaybookContents) > 0 {
setRunnerEnvVar(h, "RUNNER_PLAYBOOK", a.PlaybookContents, "playbookContents", job, hashes)
} else if len(playbook) > 0 {
} else if len(a.Playbook) > 0 {
// As we set "playbook.yaml" as default
// we need to ensure that PlaybookContents is empty before adding playbook
setRunnerEnvVar(h, "RUNNER_PLAYBOOK", playbook, "playbooks", job, hashes)
setRunnerEnvVar(h, "RUNNER_PLAYBOOK", a.Playbook, "playbooks", job, hashes)
}

if len(a.CmdLine) > 0 {
Expand Down
7 changes: 7 additions & 0 deletions tests/functional/dataplane/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ func CreateDataplaneServicesWithSameServiceType(name types.NamespacedName) {

// Create an OpenStackDataPlaneService with a given NamespacedName, and a given unstructured spec
func CreateDataPlaneServiceFromSpec(name types.NamespacedName, spec map[string]interface{}) *unstructured.Unstructured {
if spec["playbook"] == nil && spec["playbookContents"] == nil && spec["role"] == nil {
spec["playbook"] = "test"
}
raw := map[string]interface{}{

"apiVersion": "dataplane.openstack.org/v1beta1",
Expand Down Expand Up @@ -514,6 +517,9 @@ func DefaultDataplaneService(name types.NamespacedName) map[string]interface{} {
"metadata": map[string]interface{}{
"name": name.Name,
"namespace": name.Namespace,
},
"spec": map[string]interface{}{
"playbook": "test",
}}
}

Expand All @@ -531,6 +537,7 @@ func DefaultDataplaneGlobalService(name types.NamespacedName) map[string]interfa
},
"spec": map[string]interface{}{
"deployOnAllNodeSets": true,
"playbook": "test",
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ var _ = Describe("OpenstackDataplaneService Test", func() {
It("spec fields are set up", func() {
service := GetService(dataplaneServiceName)
Expect(service.Spec.DataSources).To(BeEmpty())
Expect(service.Spec.Playbook).To(BeEmpty())
Expect(service.Spec.PlaybookContents).To(BeEmpty())
Expect(service.Spec.Role).To(BeEmpty())
Expect(service.Spec.DeployOnAllNodeSets).To(BeFalse())
})
})
Expand All @@ -57,7 +58,8 @@ var _ = Describe("OpenstackDataplaneService Test", func() {
It("spec fields are set up", func() {
service := GetService(dataplaneServiceName)
Expect(service.Spec.DataSources).To(BeEmpty())
Expect(service.Spec.Playbook).To(BeEmpty())
Expect(service.Spec.PlaybookContents).To(BeEmpty())
Expect(service.Spec.Role).To(BeEmpty())
Expect(service.Spec.DeployOnAllNodeSets).To(BeTrue())
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ spec:
- ansible-runner
- run
- /runner
- -p
- playbook.yaml
- -r
- test role
- -i
- custom-img-svc-edpm-compute-no-nodes-edpm-no-nodes-custom-svc
env:
- name: RUNNER_PLAYBOOK
- name: RUNNER_ROLE
value: |2+
playbook.yaml
test role
- name: RUNNER_EXTRA_VARS
value: |2+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,7 @@ metadata:
name: custom-img-svc
spec:
openStackAnsibleEERunnerImage: example.com/repo/runner-image:latest
role:
name: "test role"
hosts: "all"
strategy: "linear"
tasks:
- name: "test task"
import_role:
name: "test role"
role: "test role"
---
apiVersion: dataplane.openstack.org/v1beta1
kind: OpenStackDataPlaneNodeSet
Expand Down
Loading