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

Clean up and renames #92

Merged
merged 3 commits into from
May 4, 2021
Merged

Conversation

jensfr
Copy link
Contributor

@jensfr jensfr commented Apr 26, 2021

- Description of the problem which is fixed/What is the use case

  • use new namespace name and refer to new quay.io repository
    We should adhere to the naming convention in OpenShift and use
    the default namespace openshift-sandboxed-containers. The same goes for
    the quay.io user/repository we use. Use the new
    quay.io/openshift_sandboxed_containers instead of
    quay.io/isolatedcontainres.

  • delete example configmap for payload. We don't have a payload any more.

- What I did

This cleans up README and example files to use the namespace
openshift-sandboxed-containers instead of sandboxed-containers-operator-system.

- How to verify it

Follow the README descripton to install/uninstall and make sure all steps work.

- Description for the changelog
Use new namespace openshift-sandboxed-containers and refer to new quay.io organisation quay.io/openshift_sandboxed_containers

@jensfr jensfr added the backport-needed Indicates this PR needs a backport to a previous release label Apr 26, 2021
Copy link

@c3d c3d left a comment

Choose a reason for hiding this comment

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

Many questions and a few suggestions ;-)

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
deploy/deploy.sh Outdated Show resolved Hide resolved
deploy/deploy.yaml Show resolved Hide resolved
deploy/deploy.yaml Show resolved Hide resolved
deploy/deploy.yaml Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@jensfr jensfr force-pushed the clean-up-and-renames branch 2 times, most recently from 141d6d6 to 137d031 Compare April 27, 2021 10:21
@jensfr
Copy link
Contributor Author

jensfr commented Apr 27, 2021

I pushed another update. The biggest change is that we're now using openshift-sandboxed-containers**-operator** as the namespace. It matches the other openshift operators.

@jensfr jensfr removed the request for review from harche April 27, 2021 10:32
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2021
@jensfr jensfr added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2021
Copy link

@c3d c3d left a comment

Choose a reason for hiding this comment

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

Looking good now.

Copy link
Contributor

@bpradipt bpradipt 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 May 3, 2021
Copy link
Contributor

@bpradipt bpradipt left a comment

Choose a reason for hiding this comment

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

/lgtm

Minor nit: Can the VERSION be defaulted to latest ?

@c3d c3d mentioned this pull request May 3, 2021
jensfr added 3 commits May 4, 2021 13:28
We should adhere to the naming convention in OpenShift and use the
default namespace openshift-sandboxed-containers-operator. The same goes
for the quay.io user/repository we use. Use the new
quay.io/openshift_sandboxed_containers instead of
quay.io/isolatedcontainres.

This patch does two things:
1. it cleans up README and example files to use the namespace
openshift-sandboxed-containers instead of
sandboxed-containers-operator-system.
2. use version 1.0 instead of 4.8

I've disabled namespacePrefix in config/default/kustomize.yaml to avoid
it to add "-system" to the namespace. This was the only way to make it
work since kustomize doesn't allow to leave the name field in the
namespace metadata blank.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
update it to match the latest state. When adding support for
the sandboxed-containers RHCOS extension we added some fields.
Those were missing in depoy.yaml and we are adding them with
this patch.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Since we are now using the RHCOS extension to install sandboxed
containers rpms we have no need for the configmap any more. There
is no daemon to use it and no payload image to refer to any more.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 4, 2021
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

Copy link

@c3d c3d left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks for splitting version and latest.

@openshift-ci-robot
Copy link

@c3d: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Thanks for splitting version and latest.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@c3d
Copy link

c3d commented May 4, 2021

Apparently I'm not a collaborator. Sill approving, for what it's worth ;-)

@jensfr jensfr removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2021
@jensfr jensfr merged commit 9050eef into openshift:master May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-needed Indicates this PR needs a backport to a previous release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants