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

"odo push" doesn't delete OpenShift objects when component name is changed #2172

Closed
dharmit opened this issue Sep 25, 2019 · 13 comments · Fixed by #2454
Closed

"odo push" doesn't delete OpenShift objects when component name is changed #2172

dharmit opened this issue Sep 25, 2019 · 13 comments · Fixed by #2454
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).

Comments

@dharmit
Copy link
Member

dharmit commented Sep 25, 2019

[kind/bug]

What versions of software are you using?

  • Operating System: all
  • Output of odo version: master

How did you run odo exactly?

  1. Created a component and pushed it.
  2. Changed the name of the component by doing odo config set name fancy-name and pushed it using odo push --config.
  3. OpenShift objects created in step 1 don't get deleted.

Actual behavior

OpenShift objects from component with old name don't get deleted

Expected behavior

Shouldn't these objects get deleted? Otherwise, how exactly will these orphaned objects be deleted? By orphaned, I mean that objects that have no corresponding odo component.

@dharmit
Copy link
Member Author

dharmit commented Sep 26, 2019

Besides changing the component name, the behaviour is ditto same when we change the project of the component using odo config set project <some-project. The OpenShift objects are untouched in the project to which the component belonged earlier.

@girishramnani
Copy link
Contributor

girishramnani commented Oct 12, 2019

I feel this is expected behaviour and if we want to have a cleanup process in place, we should ask You are changing the project/ component name, this would create an entirely new component, Do you want to delete the current component?

In my opinion the orphaned objects shouldn't be deleted without consent

@dharmit
Copy link
Member Author

dharmit commented Oct 14, 2019

I feel this is expected behaviour and if we want to have a cleanup process in place, we should ask You are changing the project/ component name, this would create an entirely new component, Do you want to delete the current component?

+1. Makes sense to me.

In my opinion the orphaned objects shouldn't be deleted without consent

And along the same lines, orphaned objects shouldn't be allowed to linger around without consent. That's what prompted me to open this issue. 😉

@sspeiche sspeiche added kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). labels Oct 14, 2019
@kadel
Copy link
Member

kadel commented Oct 15, 2019

I feel this is expected behaviour and if we want to have a cleanup process in place, we should ask You are changing the project/ component name, this would create an entirely new component, Do you want to delete the current component?

+1. Makes sense to me.

But how would you detect that the name was changed?
The only command that can display this message is odo config, we could add it there. But that won't prevent someone from changing the name the localConfig directly without using odo config command.
But I guess that we could say that in that case, it is the user's problem what he is doing there.
In this case, we need to explain in the documentation that editing localConfig directly might be dangerous.

In my opinion the orphaned objects shouldn't be deleted without consent

And along the same lines, orphaned objects shouldn't be allowed to linger around without consent. That's what prompted me to open this issue. 😉

Problem is that we can't detect if that the component was orphaned due to name or project change.

First, we need to make a list of the configuration options that can't be easily changed without side effects.

  • component name
  • project

Then we have two options

  1. show a warning when odo config set is executed. Each config option will have to have a different message explaining what is going to happen.
  2. make the config options unchangeable odo config set will complain that user can't change this value.

@mik-dass
Copy link
Contributor

mik-dass commented Oct 23, 2019

I think we should warn the user when a project or component name change is initiated and then if he proceeds with the name change, then we should delete the old component and create a new one. @kadel @cdrage @dharmit WDYT?

@dharmit
Copy link
Member Author

dharmit commented Oct 24, 2019

@kadel

The only command that can display this message is odo config, we could add it there.

+1

But that won't prevent someone from changing the name the localConfig directly without using odo config command.

We could add comments in the .odo/config.yaml just like we have them in configuration files living under /etc/ on a Linux system. So if a user tries to manually change it, they are aware that there are side-effects.

Problem is that we can't detect if that the component was orphaned due to name or project change.

All we could do is add a warning message

First, we need to make a list of the configuration options that can't be easily changed without side effects.

  • component name
  • project

Then we have two options

  1. show a warning when odo config set is executed. Each config option will have to have a different message explaining what is going to happen.
  2. make the config options unchangeable odo config set will complain that user can't change this value.

How do we make the last thing happen, i.e., odo config set will complain that user can't change this value? Do we compel it such that whenever user executes something like odo config --set project newproject, it fails with a message? 🤔

@mik-dass

I think we should warn the user when a project or component name change is initiated and then if he proceeds with the name change, then we should delete the old component and create a new one.

You mean the user should delete the old component while we already create a new one when they do odo push?

@mik-dass
Copy link
Contributor

How do we make the last thing happen, i.e., odo config set will complain that user can't change this value? Do we compel it such that whenever user executes something like odo config --set project newproject, it fails with a message? thinking
You mean the user should delete the old component while we already create a new one when they do odo push?

Yes. Whenever the user triggers odo config --set project newproject or odo config --set name newcomponent we will display a warning that this action will delete the old component and create a new one. If the user presses y, we will, in a parallel manner, create the new component and delete the old one. @dharmit @kadel @cdrage @girishramnani WDYT?

@cdrage
Copy link
Member

cdrage commented Oct 25, 2019

IMO since the user has already changed the name, I don't think we should prompt for confirmation. If you change the name of your component, you know what you're doing.

I don't think we should even warn the user, maybe in verbose output, but that's it. If changed by config, we should at least confirm that we are going to be changing the component when using odo push (confirm the component exists, it has the correct labels, etc.)

@mik-dass
Copy link
Contributor

IMO since the user has already changed the name, I don't think we should prompt for confirmation. If you change the name of your component, you know what you're doing.

@cdrage So you mean no confirmation message prompt, just delete the old component and create the new one after triggering the odo config set command?

@cdrage
Copy link
Member

cdrage commented Oct 31, 2019

@mik-dass Yes, that's correct, deleting the old component and re-creating it (in my opinion). But we have to be 100% sure that's what is intended, as to not have odo accidentally delete anything.

@mik-dass
Copy link
Contributor

mik-dass commented Nov 4, 2019

@kadel WDYT?

@dharmit
Copy link
Member Author

dharmit commented Nov 6, 2019

IMO since the user has already changed the name, I don't think we should prompt for confirmation. If you change the name of your component, you know what you're doing.
I don't think we should even warn the user

I believe, it would be helpful to have at least a prompt or a warning message. We're talking about the users who are not really aware of OpenShift. A warning message would be helpful, IMO.

@kadel
Copy link
Member

kadel commented Nov 27, 2019

Let's start asking the user to confirm when odo config set name is executed without -f
Something like this.

You are changing the component name. This action might result in the creation of a duplicate component. If your component is already pushed, please delete the <old_component_name> after you apply changes (odo component delete <old_component_name> --app <app> --project <project>)

Of course, the <something> placeholders should be replaced with right values

mik-dass added a commit to mik-dass/odo that referenced this issue Dec 12, 2019
/kind feature

What does does this PR do / why we need it:

Adds a warning message while component name or project name of the component name is changed in the config

Which issue(s) this PR fixes:

Fixes redhat-developer#2172

How to test changes / Special notes to the reviewer:

- Create a component
- Change the component name or project name in the config odo config set name <value> or odo config set project <value>
- Check if the warning message is displayed properly or not

Signed-off-by: mik-dass <mrinald7@gmail.com>
mik-dass added a commit to mik-dass/odo that referenced this issue Dec 13, 2019
/kind feature

What does does this PR do / why we need it:

Adds a warning message while component name or project name of the component name is changed in the config

Which issue(s) this PR fixes:

Fixes redhat-developer#2172

How to test changes / Special notes to the reviewer:

- Create a component
- Change the component name or project name in the config odo config set name <value> or odo config set project <value>
- Check if the warning message is displayed properly or not

Signed-off-by: mik-dass <mrinald7@gmail.com>
mik-dass added a commit to mik-dass/odo that referenced this issue Dec 16, 2019
/kind feature

What does does this PR do / why we need it:

Adds a warning message while component name or project name of the component name is changed in the config

Which issue(s) this PR fixes:

Fixes redhat-developer#2172

How to test changes / Special notes to the reviewer:

- Create a component
- Change the component name or project name in the config odo config set name <value> or odo config set project <value>
- Check if the warning message is displayed properly or not

Signed-off-by: mik-dass <mrinald7@gmail.com>
mik-dass added a commit to mik-dass/odo that referenced this issue Dec 17, 2019
/kind feature

What does does this PR do / why we need it:

Adds a warning message while component name or project name of the component name is changed in the config

Which issue(s) this PR fixes:

Fixes redhat-developer#2172

How to test changes / Special notes to the reviewer:

- Create a component
- Change the component name or project name in the config odo config set name <value> or odo config set project <value>
- Check if the warning message is displayed properly or not

Signed-off-by: mik-dass <mrinald7@gmail.com>
openshift-merge-robot pushed a commit that referenced this issue Dec 17, 2019
/kind feature

What does does this PR do / why we need it:

Adds a warning message while component name or project name of the component name is changed in the config

Which issue(s) this PR fixes:

Fixes #2172

How to test changes / Special notes to the reviewer:

- Create a component
- Change the component name or project name in the config odo config set name <value> or odo config set project <value>
- Check if the warning message is displayed properly or not

Signed-off-by: mik-dass <mrinald7@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants