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.

Sign up for GitHub

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

Jump to bottom

new flag --listen-apiserver-port for docker/podman: allow user to specify host port for api-server #11070

Closed
wants to merge 3 commits into from
Closed

new flag --listen-apiserver-port for docker/podman: allow user to specify host port for api-server #11070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da0cdb9
Add listen-apiserver-port for docker driver
zhan9san Apr 12, 2021
11beee3
Add validation of ListenAPIServerPort
zhan9san Apr 20, 2021
19dacad
Ensure that the socket is always closed
zhan9san Apr 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions cmd/minikube/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,10 @@ func validateFlags(cmd *cobra.Command, drvName string) {
validateListenAddress(viper.GetString(listenAddress))
}

if cmd.Flags().Changed(listenAPIServerPort) {
validateListenAPIServerPort(viper.GetInt(listenAPIServerPort))
}

if cmd.Flags().Changed(imageRepository) {
viper.Set(imageRepository, validateImageRepository(viper.GetString(imageRepository)))
}
Expand Down Expand Up @@ -1244,6 +1248,20 @@ func validateListenAddress(listenAddr string) {
}
}

// This function validates if the --listen-apiserver-port
// is valid
func validateListenAPIServerPort(listenAPIServerPort int) {
if listenAPIServerPort < 0 || listenAPIServerPort > 65535 {
exit.Message(reason.Usage, "Sorry, the port provided with the --listen-apiserver-port flag is invalid: {{.listenAPIServerPort}}.", out.V{"listenAPIServerPort": listenAPIServerPort})
} else {
ln, err := net.Listen("tcp", fmt.Sprintf(":%d", listenAPIServerPort))
if err != nil {
exit.Message(reason.Usage, "Sorry, the port provided with the --listen-apiserver-port flag is already allocated: {{.listenAPIServerPort}}.", out.V{"listenAPIServerPort": listenAPIServerPort})
}
defer ln.Close()
Copy link
Member

Choose a reason for hiding this comment

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

this defer needs to happen before Exit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @medyagh
Thanks so much for your time.

If it goes into exit.Message, that means the socket wouldn't be created. Do we still need to modify this snippet?
You're far more familiar to Golang that me. I referred the logic in this file, example_test.go, at line 23.

Besides, I noticed #9404 has achieved this feature. The only drawback is that #9404 exposes one more random port.
Should we still work on this feature, --listen-apiserver-port?

Please let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

@zhan9san You are correct about defer, if an error is returned no action is required on the socket.

}
}

// This function validates that the --insecure-registry follows one of the following formats:
// "<ip>[:<port>]" "<hostname>[:<port>]" "<network>/<netmask>"
func validateInsecureRegistry() {
Expand Down
3 changes: 3 additions & 0 deletions cmd/minikube/cmd/start_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const (
defaultSSHUser = "root"
defaultSSHPort = 22
listenAddress = "listen-address"
listenAPIServerPort = "listen-apiserver-port"
)

var (
Expand Down Expand Up @@ -219,6 +220,7 @@ func initDriverFlags() {

// docker & podman
startCmd.Flags().String(listenAddress, "", "IP Address to use to expose ports (docker and podman driver only)")
startCmd.Flags().Int(listenAPIServerPort, 0, "Port that apiserver exposed (docker and podman driver only). Use it with --listen-address=0.0.0.0(for remote access) --apiserver-ips=HostIP(for certificate).")
startCmd.Flags().StringSlice(ports, []string{}, "List of ports that should be exposed (docker and podman driver only)")
}

Expand Down Expand Up @@ -369,6 +371,7 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s
DiskSize: diskSize,
Driver: drvName,
ListenAddress: viper.GetString(listenAddress),
ListenAPIServerPort: viper.GetInt(listenAPIServerPort),
HyperkitVpnKitSock: viper.GetString(vpnkitSock),
HyperkitVSockPorts: viper.GetStringSlice(vsockPorts),
NFSShare: viper.GetStringSlice(nfsShare),
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
github.com/hectane/go-acl v0.0.0-20190604041725-da78bae5fc95 // indirect
github.com/hooklift/assert v0.0.0-20170704181755-9d1defd6d214 // indirect
github.com/hooklift/iso9660 v0.0.0-20170318115843-1cf07e5970d8
github.com/intel-go/cpuid v0.0.0-20181003105527-1a4a6f06a1c6
github.com/intel-go/cpuid v0.0.0-20181003105527-1a4a6f06a1c6 // indirect
github.com/johanneswuerbach/nfsexports v0.0.0-20200318065542-c48c3734757f
github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c
github.com/juju/errors v0.0.0-20190806202954-0232dcc7464d // indirect
Expand Down
1 change: 0 additions & 1 deletion go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,6 @@ github.com/opencontainers/selinux v1.8.0/go.mod h1:RScLhm78qiWa2gbVCcGkC7tCGdgk3
github.com/otiai10/copy v1.5.0 h1:SoXDGnlTUZoqB/wSuj/Y5L6T5i6iN4YRAcMCd+JnLNU=
github.com/otiai10/copy v1.5.0/go.mod h1:XWfuS3CrI0R6IE0FbgHsEazaXO8G0LpMp9o8tos0x4E=
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
github.com/otiai10/curr v1.0.0 h1:TJIWdbX0B+kpNagQrjgq8bCMrbhiuX73M2XwgtDMoOI=
github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs=
github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo=
github.com/otiai10/mint v1.3.2 h1:VYWnrP5fXmz1MXvjuUvcBrXSjGE6xjON+axB/UrpO3E=
Expand Down
6 changes: 6 additions & 0 deletions pkg/drivers/kic/kic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,16 @@ func (d *Driver) Create() error {
listAddr = "0.0.0.0"
}

listenAPIServerPort := 0
if d.NodeConfig.ListenAPIServerPort != 0 {
listenAPIServerPort = d.NodeConfig.ListenAPIServerPort
}

// control plane specific options
params.PortMappings = append(params.PortMappings,
oci.PortMapping{
ListenAddress: listAddr,
HostPort: int32(listenAPIServerPort),
ContainerPort: int32(params.APIServerPort),
},
oci.PortMapping{
Expand Down
9 changes: 7 additions & 2 deletions pkg/drivers/kic/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,13 @@ func generatePortMappings(portMappings ...PortMapping) []string {
result := make([]string, 0, len(portMappings))
for _, pm := range portMappings {
// let docker pick a host port by leaving it as ::
// example --publish=127.0.0.17::8443 will get a random host port for 8443
publish := fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort)
// example --publish=127.0.0.17:8443:8443 will get a fixed host port for 8443
publish := ""
if pm.HostPort != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

how about add a Check here to verify this port is Free and Usable and if it is not Free and usable return an error to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's OK.
I'll add this validation based on what docker does.

publish = fmt.Sprintf("--publish=%s:%d:%d", pm.ListenAddress, pm.HostPort, pm.ContainerPort)
} else {
publish = fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort)
}
result = append(result, publish)
}
return result
Expand Down
33 changes: 17 additions & 16 deletions pkg/drivers/kic/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,21 @@ var (

// Config is configuration for the kic driver used by registry
type Config struct {
ClusterName string // The cluster the container belongs to
MachineName string // maps to the container name being created
CPU int // Number of CPU cores assigned to the container
Memory int // max memory in MB
StorePath string // libmachine store path
OCIBinary string // oci tool to use (docker, podman,...)
ImageDigest string // image name with sha to use for the node
Mounts []oci.Mount // mounts
APIServerPort int // Kubernetes api server port inside the container
PortMappings []oci.PortMapping // container port mappings
Envs map[string]string // key,value of environment variables passed to the node
KubernetesVersion string // Kubernetes version to install
ContainerRuntime string // container runtime kic is running
Network string // network to run with kic
ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080...
ListenAddress string // IP Address to listen to
ClusterName string // The cluster the container belongs to
MachineName string // maps to the container name being created
CPU int // Number of CPU cores assigned to the container
Memory int // max memory in MB
StorePath string // libmachine store path
OCIBinary string // oci tool to use (docker, podman,...)
ImageDigest string // image name with sha to use for the node
Mounts []oci.Mount // mounts
APIServerPort int // Kubernetes api server port inside the container
PortMappings []oci.PortMapping // container port mappings
Envs map[string]string // key,value of environment variables passed to the node
KubernetesVersion string // Kubernetes version to install
ContainerRuntime string // container runtime kic is running
Network string // network to run with kic
ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080...
ListenAddress string // IP Address to listen to
ListenAPIServerPort int // apiserver Port to listen to
}
1 change: 1 addition & 0 deletions pkg/minikube/config/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type ClusterConfig struct {
ScheduledStop *ScheduledStopConfig
ExposedPorts []string // Only used by the docker and podman driver
ListenAddress string // Only used by the docker and podman driver
ListenAPIServerPort int // Only used by the docker and podman driver
Network string // only used by docker driver
MultiNodeRequested bool
}
Expand Down
29 changes: 15 additions & 14 deletions pkg/minikube/registry/drvs/docker/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,21 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) {
}

return kic.NewDriver(kic.Config{
ClusterName: cc.Name,
MachineName: config.MachineName(cc, n),
StorePath: localpath.MiniPath(),
ImageDigest: cc.KicBaseImage,
Mounts: mounts,
CPU: cc.CPUs,
Memory: cc.Memory,
OCIBinary: oci.Docker,
APIServerPort: cc.Nodes[0].Port,
KubernetesVersion: cc.KubernetesConfig.KubernetesVersion,
ContainerRuntime: cc.KubernetesConfig.ContainerRuntime,
ExtraArgs: extraArgs,
Network: cc.Network,
ListenAddress: cc.ListenAddress,
ClusterName: cc.Name,
MachineName: config.MachineName(cc, n),
StorePath: localpath.MiniPath(),
ImageDigest: cc.KicBaseImage,
Mounts: mounts,
CPU: cc.CPUs,
Memory: cc.Memory,
OCIBinary: oci.Docker,
APIServerPort: cc.Nodes[0].Port,
KubernetesVersion: cc.KubernetesConfig.KubernetesVersion,
ContainerRuntime: cc.KubernetesConfig.ContainerRuntime,
ExtraArgs: extraArgs,
Network: cc.Network,
ListenAddress: cc.ListenAddress,
ListenAPIServerPort: cc.ListenAPIServerPort,
}), nil
}

Expand Down
27 changes: 14 additions & 13 deletions pkg/minikube/registry/drvs/podman/podman.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,20 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) {
}

return kic.NewDriver(kic.Config{
ClusterName: cc.Name,
MachineName: config.MachineName(cc, n),
StorePath: localpath.MiniPath(),
ImageDigest: strings.Split(cc.KicBaseImage, "@")[0], // for podman does not support docker images references with both a tag and digest.
Mounts: mounts,
CPU: cc.CPUs,
Memory: cc.Memory,
OCIBinary: oci.Podman,
APIServerPort: cc.Nodes[0].Port,
KubernetesVersion: cc.KubernetesConfig.KubernetesVersion,
ContainerRuntime: cc.KubernetesConfig.ContainerRuntime,
ExtraArgs: extraArgs,
ListenAddress: cc.ListenAddress,
ClusterName: cc.Name,
MachineName: config.MachineName(cc, n),
StorePath: localpath.MiniPath(),
ImageDigest: strings.Split(cc.KicBaseImage, "@")[0], // for podman does not support docker images references with both a tag and digest.
Mounts: mounts,
CPU: cc.CPUs,
Memory: cc.Memory,
OCIBinary: oci.Podman,
APIServerPort: cc.Nodes[0].Port,
KubernetesVersion: cc.KubernetesConfig.KubernetesVersion,
ContainerRuntime: cc.KubernetesConfig.ContainerRuntime,
ExtraArgs: extraArgs,
ListenAddress: cc.ListenAddress,
ListenAPIServerPort: cc.ListenAPIServerPort,
}), nil
}

Expand Down
1 change: 1 addition & 0 deletions site/content/en/docs/commands/start.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ minikube start [flags]
--kvm-numa-count int Simulate numa node count in minikube, supported numa node count range is 1-8 (kvm2 driver only) (default 1)
--kvm-qemu-uri string The KVM QEMU connection URI. (kvm2 driver only) (default "qemu:///system")
--listen-address string IP Address to use to expose ports (docker and podman driver only)
--listen-apiserver-port int Port that apiserver exposed (docker and podman driver only). Use it with --listen-address=0.0.0.0(for remote access) --apiserver-ips=HostIP(for certificate).
--memory string Amount of RAM to allocate to Kubernetes (format: <number>[<unit>], where unit = b, k, m or g).
--mount This will start the mount daemon and automatically mount files into minikube.
--mount-string string The argument to pass the minikube mount command on start.
Expand Down