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

Deprecated inotify-tools on Ansible based-operator images and remove Ansible sidecar container. #2586

Merged
merged 2 commits into from
Feb 21, 2020
Merged

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Feb 21, 2020

Description of the change:

  • Improve the log in the operator to add the event stats
  • Remove the ansible sidecar container used just to provide the Ansible logs.
  • deprecated the inotify-tools usage in order to be able to remove it before 1.0.0

Motivation for the change:

Closes #2007

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 21, 2020
@camilamacedo86 camilamacedo86 changed the title remove ansible containers scaffolded by default Remove a dependency on inotify-tools from Ansible based-operator images and the additional Ansible side-card container. Feb 21, 2020
@camilamacedo86 camilamacedo86 changed the title Remove a dependency on inotify-tools from Ansible based-operator images and the additional Ansible side-card container. Remove a dependency on inotify-tools from Ansible based-operator images and the additional Ansible sidecar container. Feb 21, 2020
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

There's a small typo in the developer guide. The code makes sense. I'd like @fabianvf to give it a closer look.

doc/ansible/dev/developer_guide.md Outdated Show resolved Hide resolved
Copy link
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

+1 to dropping the container from the scaffolding, but I think we should leave the script + script scaffolding in place for the time being (and maybe add in a deprecation warning or something). If we remove it right away it will break any operator deployment that uses the old scaffolding, but it's easy to keep it around for a few versions with a warning before dropping it completely. Even if we replaced the whole script with just an echoed This script is deprecated and will soon be removed it would at least stop deployments from failing completely. Thoughts?

hack/image/ansible/scaffold-ansible-image.go Show resolved Hide resolved
@@ -1,52 +0,0 @@
// Copyright 2019 The Operator-SDK Authors
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave this here for now

```yaml
- name: {{your operator name which is the value of metadata.name in this file}}
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@estroz I followed here your suggestion.
I will try to start to do that as well.
Let's see if it can build a good example/case in the order we start to do it for all.

const aoLogsTmpl = `#!/bin/bash

echo "WARN: This script is deprecated and will soon be removed"

watch_dir=${1:-/tmp/ansible-operator/runner}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2020-02-21 at 20 02 50

c/c @fabianvf

Copy link
Member

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2020
@camilamacedo86 camilamacedo86 changed the title Remove a dependency on inotify-tools from Ansible based-operator images and the additional Ansible sidecar container. Deprecated inotify-tools on Ansible based-operator images and remove Ansible sidecar container. Feb 21, 2020
@camilamacedo86 camilamacedo86 merged commit 1ea7773 into operator-framework:master Feb 21, 2020
@camilamacedo86 camilamacedo86 deleted the remove-ansible-container branch February 21, 2020 20:47
camilamacedo86 added a commit that referenced this pull request Feb 25, 2020
…le logs on it (#2589)

**Description of the change:**

- Add full Ansible result output to the operator logs for Ansible based-operators configurable by EnvVar.

**Motivation for the change:**

Allow users to have the same full information that can be obtained until the version 0.15.x with the Ansible sidecar container in the operator logs. 

Note that we deprecated the inotify-tools and we will no longer scaffold the sidecar container. See #2586. Also, we have been improving the operator logs in order to attend all needs. See: #2580 and #2321.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. 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.

Remove the need of the usage of inotify-tools since it is no longer provide in the rhel8
4 participants