Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

Allow excluding IoT CRDs during Ansible installation (fixes #5046) #5054

Closed
wants to merge 2 commits into from
Closed

Allow excluding IoT CRDs during Ansible installation (fixes #5046) #5054

wants to merge 2 commits into from

Conversation

ctron
Copy link
Contributor

@ctron ctron commented Jul 23, 2020

Type of change

  • Enhancement / new feature

Description

Checklist

  • Update/write design documentation in ./documentation/design
  • Write tests and make sure they pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md

@ctron ctron requested a review from k-wall July 23, 2020 11:54
@ctron ctron marked this pull request as draft July 23, 2020 11:55
@ctron ctron linked an issue Jul 23, 2020 that may be closed by this pull request
Ability to exclude EnMasse IoT components from Ansible install #5046
Open
@k-wall
with -> when
723b007 
default to install iot true (maintains existing behaviour)
exclude crds directory itself from the find results
@k-wall
Copy link
Member

k-wall commented Jul 24, 2020

@ctron tested locally and had to fix a couple of bugs. it would for me now.

@kornys
Copy link
Member

kornys commented Jul 24, 2020

right now this PR does not makes sense why we set defaults to enable iot?

@k-wall
Copy link
Member

k-wall commented Jul 29, 2020

right now this PR does not makes sense why we set defaults to enable iot?

why would the default be to disable? If I install from yaml bundle or OLM, I get IoT. If I install from Ansible, I would expect the same. We've added the option so that users of Ansible can elect to disable. I don't think it should be the default.

@kornys
Copy link
Member

kornys commented Jul 29, 2020

@k-wall because ansible is used only by integratly I think, and for testing they use our defaults inventory files, so it will cause troubles I guess if we change old behavior

@k-wall
Copy link
Member

k-wall commented Jul 30, 2020 •
edited

@kornys - I'm just arguing against a change in the behaviour of of our release. Previous releases defaulted to install IoT, so we should not break that. If users such as Integrately need to disable the IoT install, they will have to create their own inventory - or pass additional options to the ansible-playbook command line to inject additional parameters. That doesn't seem burdensome or unreasonable to me.

@lulf
Copy link
Member

lulf commented Aug 17, 2020

Release-0.33 branch was renamed to master: Please re-create PR against master branch.

@lulf lulf closed this Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@k-wall k-wall k-wall approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to exclude EnMasse IoT components from Ansible install
4 participants
@ctron @k-wall @kornys @lulf