podman http api client #51

towe75 · 2020-07-20T19:43:20Z

Goal

Goal of this PR is to replace the deprecated podman varlink api client with a http/rest based implementation.
It will not be a generic podman client, structs and methods will only implement the minimum needed attributes/features.

Todo

Client implementation

Automatically discover root and user-specific domain socket path
Configurable unix domain socket or http server path
Retry remote requests in case of network outages or known non-final responses

Endpoints

Port currently implemented varlink endpoints:

Buildsystem

ensure we can use the http api via github action runner

Port features

Ensure clean ENV in rootless mode (find alternative to older pull image approach)

Improvements

Improve GH Actions: run complete testsuite additionally in rootless mode
Stream Container Stats instead of polling

mheon · 2020-07-31T19:27:11Z

Have you considered using the Golang API bindings in the Podman repo here?

towe75 · 2020-08-01T12:18:37Z

@mheon , yes, we discussed about it in #37 and created also a podman issue at containers/podman#6866

We would like, of course, to reuse some of your work but hesitate to do it. Our concerns are:

huge dependency because the binding is not separated in a go module
somewhat unclear if this becomes the "official" client and is maintained for a longer time - hence podman issue 6866

Thank you for reaching out , it would be nice if you can clarify the intentions about your podman binding package.

Start http api service unit

Improved container startup and stats Disabled oom test because of podman problems

Enable filesystem isolation mode, solved weird Env[] problems Expose more podman systeminfo details to nomad Removed custom cgroupv2 detector, rely on podman instead

drewbailey

@towe75 this is amazing work! I have many stylistic nitpicks and only a few questions. Really awesome job getting this together. Let me know if you have any questions.

drewbailey · 2020-11-05T19:22:34Z

apiclient/apiclient.go

+	logger     hclog.Logger
+}
+
+func NewClient(logger hclog.Logger) *APIClient {


GuessSocketPath sounds like we are guessing :) Thoughts on passing in a an Config struct, that contains a logger and socket path? when we initialize the config we can use a renamed version of GuessSocketPath as DefaultSocketPath is one isn't provided

Method is now renamed to DefaultSocketPath().

Not sure how to approach the config struct. What about moving api initialization from NewPodmanDriver() to SetConfig()? This way we could simply share the drivers config with api or derive a separate struct from driverconfig.

IMO something like this would be nice, if SetConfig is the entry point it could be built up there, We can provide a DefaultConfig() method in the api package if we are initializing the client first, then calling setconfig but that seems like something to think about for future improvements to only call once

type ClientConfig struct{ SocketPath string } func NewClient(cfg ClientConfig, logger hclog.Logger) *APIClient {

apiclient/apiclient.go

drewbailey · 2020-11-05T19:24:54Z

apiclient/apiclient.go

+func (c *APIClient) SetSocketPath(baseUrl string) {
+	c.logger.Debug("http baseurl", "url", baseUrl)
+	c.httpClient = &http.Client{
+		Timeout: 60 * time.Second,


Timeout seems like another good candidate to make configurable

apiclient/apiclient.go

apiclient/container_inspect.go

driver.go

drewbailey · 2020-11-05T19:50:16Z

driver.go

+		} else if driverConfig.NetworkMode == "slirp4netns" {
+			createOpts.ContainerNetworkConfig.NetNS.NSMode = apiclient.Slirp
+		} else {
+			// FIXME: needs more work, parsing etc.


can we track this with a github issue

This "else" part is a typical place to handle not-yet-known requirements. I would not know what to write into the issue except "implement all possible podman network modes and switches". We can remove the FIXME because it's not a bug per se.

driver.go

driver_test.go

drewbailey

APL2 headers should be removed, repo is under MPL, no file specific headers necessary

apiclient/container_start.go

renamed apiclient package to api improved httpclient context usage improved driver struct, renamed field podmanClient2 to podman

Renamed GuessSocketPath to DefaultSocketPath Removed api-version constant Cleaned up api-endpoints, do not use "compat" api, always send api version Worked around flaky tests by adding sleeps to avoid very shortliving containers

Improved comments Simplified TestPodmanDriver_SignalTask

api/container_delete.go

api/container_create.go

Moved all podman structs to new file structs.go

Moved api.NewClient to driver.SetConfig()

towe75 · 2020-11-15T10:38:35Z

@drewbailey PTAL. Everything should be resolved now.

drewbailey

Looking really good, just a few outstanding comments and questions left and then should be good to go!

drewbailey · 2020-11-16T14:01:20Z

api/container_create.go

+	if err != nil {
+		return response, err
+	}
+	// fmt.Println(string(jsonString))


Suggested change

// fmt.Println(string(jsonString))

drewbailey · 2020-11-16T14:03:07Z

api/container_start.go

+	}
+
+	// wait max 10 seconds for running state
+	timeout, cancel := context.WithTimeout(ctx, time.Second*10)


Suggested change

timeout, cancel := context.WithTimeout(ctx, time.Second*10)

// TODO make timeout configurable

timeout, cancel := context.WithTimeout(ctx, time.Second*10)

drewbailey · 2020-11-16T14:04:06Z

api/system_info.go

+
+	var infoData Info
+
+	// the libpod/info endpoint seems to have some trouble


Suggested change

// the libpod/info endpoint seems to have some trouble

// The libpod/info endpoint seems to have some trouble

Will remove this stale comment, we're not using the compat endpoint anymore.

drewbailey · 2020-11-16T14:05:31Z

driver.go

+This Source Code Form is subject to the terms of the Mozilla Public
+License, v. 2.0. If a copy of the MPL was not distributed with this
+file, You can obtain one at https://mozilla.org/MPL/2.0/.
 */


Suggested change

This Source Code Form is subject to the terms of the Mozilla Public

License, v. 2.0. If a copy of the MPL was not distributed with this

file, You can obtain one at https://mozilla.org/MPL/2.0/.

*/

drewbailey · 2020-11-16T14:06:30Z

driver.go

@@ -341,6 +335,8 @@ func BuildContainerName(cfg *drivers.TaskConfig) string {

 // StartTask creates and starts a new Container based on the given TaskConfig.
 func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drivers.DriverNetwork, error) {
+	// d.logger.Warn("Env1", "env1", cfg.Env)


re-bumping this :)

drewbailey · 2020-11-16T14:07:19Z

driver.go

-		cgroupv2 := false
-		for _, l := range procFilesystems {
-			cgroupv2 = cgroupv2 || strings.HasSuffix(l, "cgroup2")
+	if cfg.Resources.NomadResources.Memory.MemoryMB > 0 {


what is this limit? MemoryMB * 1024 * 1024 seems high, unless memoryMB its bytes?

driver.go

drewbailey · 2020-11-16T14:08:43Z

handle.go

+This Source Code Form is subject to the terms of the Mozilla Public
+License, v. 2.0. If a copy of the MPL was not distributed with this
+file, You can obtain one at https://mozilla.org/MPL/2.0/.
 */


Suggested change

This Source Code Form is subject to the terms of the Mozilla Public

License, v. 2.0. If a copy of the MPL was not distributed with this

file, You can obtain one at https://mozilla.org/MPL/2.0/.

*/

drewbailey · 2020-11-16T14:08:59Z

main.go

+This Source Code Form is subject to the terms of the Mozilla Public
+License, v. 2.0. If a copy of the MPL was not distributed with this
+file, You can obtain one at https://mozilla.org/MPL/2.0/.
 */


Suggested change

This Source Code Form is subject to the terms of the Mozilla Public

License, v. 2.0. If a copy of the MPL was not distributed with this

file, You can obtain one at https://mozilla.org/MPL/2.0/.

*/

drewbailey · 2020-11-16T14:09:08Z

state.go

+This Source Code Form is subject to the terms of the Mozilla Public
+License, v. 2.0. If a copy of the MPL was not distributed with this
+file, You can obtain one at https://mozilla.org/MPL/2.0/.
 */


Suggested change

This Source Code Form is subject to the terms of the Mozilla Public

License, v. 2.0. If a copy of the MPL was not distributed with this

file, You can obtain one at https://mozilla.org/MPL/2.0/.

*/

towe75 added 3 commits July 20, 2020 15:33

podman http api client initial commit

eeb5c43

Merge branch 'master' into f-http-api

8c7b915

Implement ContainerStart http api call

951c53a

towe75 added enhancement labels Jul 31, 2020

towe75 added 23 commits August 2, 2020 10:29

Implement ContailerDelete http api call

e9d30e1

Switch podman repo to opensuse

d0591f2

Start http api service unit

Implement ContainerInspect and SystemInfo http api calls

de69df0

Linter

ab0bcba

License headers

5d363e6

linter, fmt

4ac65cf

Remove separate go setup, rely on base image

6f05360

Makefile improvements

b2c8e24

implemented api v2 create container

4ca9b04

Improved container startup and stats Disabled oom test because of podman problems

Merged SignalTask support and linter changes

819cd82

Improved tmpfs unittest

84bb809

Implement ContainerKill / SignalTask http api call

f06c899

Fix SignalTask unittest

c9bb044

Implement ContainerStats http api call

34807fd

Code cleanup

220228c

Removed varlink from tests

745fa54

Remove varlink code

dac3c0d

Fix test, lint

c394552

Pull image before create container

d49b67f

Fix ContainerInstpect, adopt some loglevels

4a5c6ee

Fixed various bugs

b2cb7a3

Enable filesystem isolation mode, solved weird Env[] problems Expose more podman systeminfo details to nomad Removed custom cgroupv2 detector, rely on podman instead

Comments

d1eddc5

Merge branch 'master' into f-http-api

3241eb9

drewbailey reviewed Nov 5, 2020

View reviewed changes

drewbailey suggested changes Nov 5, 2020

View reviewed changes

apiclient/container_start.go Outdated Show resolved Hide resolved

towe75 added 4 commits November 6, 2020 10:23

Code review #51

205f71e

renamed apiclient package to api improved httpclient context usage improved driver struct, renamed field podmanClient2 to podman

Code review #51

48050ff

Renamed GuessSocketPath to DefaultSocketPath Removed api-version constant Cleaned up api-endpoints, do not use "compat" api, always send api version Worked around flaky tests by adding sleeps to avoid very shortliving containers

Change license headers to MPL-2

1e6ba5a

Code review #51

072b63b

Improved comments Simplified TestPodmanDriver_SignalTask

towe75 requested a review from drewbailey November 6, 2020 20:18

drewbailey reviewed Nov 9, 2020

View reviewed changes

api/container_delete.go Outdated Show resolved Hide resolved

drewbailey reviewed Nov 9, 2020

View reviewed changes

api/container_create.go Outdated Show resolved Hide resolved

towe75 added 2 commits November 9, 2020 20:30

Removed license headers completely

7f6d2f2

Code review #51

c952215

Moved all podman structs to new file structs.go

computator mentioned this pull request Nov 14, 2020

Docker API Compatibility. containers/podman#8329

Closed

Added a ClientConfig struct with default values

4e201ba

Moved api.NewClient to driver.SetConfig()

drewbailey reviewed Nov 16, 2020

View reviewed changes

Review #51

f52f9ee

drewbailey marked this pull request as ready for review November 25, 2020 18:17

drewbailey approved these changes Nov 25, 2020

View reviewed changes

towe75 changed the title ~~WIP podman http api client~~ podman http api client Nov 25, 2020

towe75 merged commit b580f2d into master Nov 25, 2020

towe75 deleted the f-http-api branch November 25, 2020 18:21

drewbailey mentioned this pull request Dec 16, 2020

Upgrade to V2 API #37

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

podman http api client #51

podman http api client #51

towe75 commented Jul 20, 2020 •

edited

Loading

mheon commented Jul 31, 2020

towe75 commented Aug 1, 2020

drewbailey left a comment

drewbailey Nov 5, 2020

towe75 Nov 6, 2020

drewbailey Nov 9, 2020

drewbailey Nov 5, 2020

drewbailey Nov 5, 2020

towe75 Nov 6, 2020

drewbailey left a comment

towe75 commented Nov 15, 2020

drewbailey left a comment

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

towe75 Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

drewbailey Nov 16, 2020

	timeout, cancel := context.WithTimeout(ctx, time.Second*10)
	// TODO make timeout configurable
	timeout, cancel := context.WithTimeout(ctx, time.Second*10)


		var infoData Info

		// the libpod/info endpoint seems to have some trouble

podman http api client #51

podman http api client #51

Conversation

towe75 commented Jul 20, 2020 • edited Loading

Goal

Todo

Client implementation

Endpoints

Buildsystem

Port features

Improvements

mheon commented Jul 31, 2020

towe75 commented Aug 1, 2020

drewbailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewbailey left a comment

Choose a reason for hiding this comment

towe75 commented Nov 15, 2020

drewbailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

towe75 commented Jul 20, 2020 •

edited

Loading