-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
rename "none" driver to "native" #7772
Comments
No, it is just out of context... I think this has been described in multiple places, but this is because it was changed from machine: I'll copy it here (from the "generic" discussion, another legacy driver name used by none
generic
machine
Now that we have forked the "none" driver, we can rename it like we did with the "kvm2" driver. But naming things is hard, even if we long considered renaming it as the "libvirt" driver instead. This is also partly due to #4829 We lack technical documentation... So, naming. The problem is that "BareMetal" is also a horrible name. There are two things wrong with it: The first one is philosophical, and that is that bare metal usually came before the OS itself. The second one is that it can be very confusing, when you run the "none" driver on a VM ? So we could use some other name, perhaps something like "Local" (as opposed to "Remote") The "none" driver could be the "local" driver, and the "generic" driver could be the "remote" driver. Another aspect of none is that you probably should be using docker/podman for some isolation. You have people running all kinds of things on the master, and failing to read the documentation... We added a link to the original documentation now, but the experience is still quite bad outside of CI (Added in |
How about |
Other alternatives:
I feel like a single-word name may be easier to deal with, particularly for non-native speakers, I'm just not sure what it should be. |
I like native |
@medyagh : I guess we push this to And it would also give me some time to do yet another attempt to implement the remote driver, formerly known as "generic" |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/assign |
@medyagh do we want to rename all of the references of none to native? Or we can just add an alias native for none, so If we do want to rename none, one thing we need to handle carefully is existing clusters using none driver. Even with alias we store the cluster driver its real name, and validates that in next start. We might need to add support so that it doesn't break anything. Anyway I am open to do both, let me know what you prefer. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
@sadlil if you are still working on this, pls let us know, or we might mark this issue as frozen |
@prezha Yes, i will work a little bit more on this. Creating the alias now. |
When the driver code is renamed, we can change "driver.BareMetal" (driver.IsNone?) to "driver.IsNative" The current is a bit confusing, it will return true when running on a VM but false when running in Docker... // BareMetal returns if this driver is unisolated
func BareMetal(name string) bool {
return name == None || name == Mock
}
// IsSSH checks if the driver is ssh
func IsSSH(name string) bool {
return name == SSH
} |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
As the above PR is closed now , is there any other person working on this issue? |
I see that there is an agreement on changing the name and working on this by maintainers here as well. This issue should not get closed. |
none is a terrible name.
and doesnt look good at least in integration tests.I suggest we add an alias to none driver.
we could do it in two phases:
change all the current integration tests names to BareMetal
in second phase, if we add an alias, so both none or barmetal be accepted.
The text was updated successfully, but these errors were encountered: