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

Cleanup in examples + fedora example #554

Merged
merged 3 commits into from
Jan 17, 2018
Merged

Cleanup in examples + fedora example #554

merged 3 commits into from
Jan 17, 2018

Conversation

jellonek
Copy link
Contributor

@jellonek jellonek commented Jan 16, 2018

Please note that additional fedora example requires fix in cloud init network data generation (will be included in next PR).


This change is Reviewable

@pigmej
Copy link
Contributor

pigmej commented Jan 16, 2018

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions.


examples/cirros-vm-with-additional-annotations.yaml, line 51 at r1 (raw file):

Quoted 7 lines of code… > # Virtlet currently ignores image tags, but their meaning may change > # in future, so it’s better not to set them for VM pods. If there’s no tag > # provided in the image specification kubelet defaults to > # imagePullPolicy: Always, which means that the image is always > # redownloaded when the pod is created. In order to make pod creation > # faster and more reliable, we set imagePullPolicy to IfNotPresent here > # so a previously downloaded image is reused if there is one

Do we need it there? Why not to put in in docs? Or at least make it shorter


examples/fedora-vm-with-testuser.yaml, line 15 at r1 (raw file):

        groups: users
        lock_passwd: false
        passwd: "$6$rounds=4096$wPs4Hz4tfs$a8ssMnlvH.3GX88yxXKF2cKMlVULsnydoOKgkuStTErTq2dzKZiIx9R/pPWWh5JLxzoZEx7lsSX5T2jW5WISi1"

can we have info what PW is that in a comment?


examples/ubuntu-vm-with-testuser.yaml, line 17 at r1 (raw file):

        groups: users
        lock_passwd: false
        passwd: "$6$rounds=4096$wPs4Hz4tfs$a8ssMnlvH.3GX88yxXKF2cKMlVULsnydoOKgkuStTErTq2dzKZiIx9R/pPWWh5JLxzoZEx7lsSX5T2jW5WISi1"

can we have plain text password as comment there?


Comments from Reviewable

It requires a fix in cloud init network data generation which will be
included in separate commit.
@jellonek jellonek force-pushed the jell/examples_cleanup branch 2 times, most recently from 530f45b to d8ec8d7 Compare January 16, 2018 16:06
@jellonek
Copy link
Contributor Author

Review status: 0 of 10 files reviewed at latest revision, 3 unresolved discussions.


examples/cirros-vm-with-additional-annotations.yaml, line 51 at r1 (raw file):

Previously, pigmej (Jędrzej Nowak) wrote…
# Virtlet currently ignores image tags, but their meaning may change
# in future, so it’s better not to set them for VM pods. If there’s no tag
# provided in the image specification kubelet defaults to
# imagePullPolicy: Always, which means that the image is always
# redownloaded when the pod is created. In order to make pod creation
# faster and more reliable, we set imagePullPolicy to IfNotPresent here
# so a previously downloaded image is reused if there is one

Do we need it there? Why not to put in in docs? Or at least make it shorter

Done.


examples/fedora-vm-with-testuser.yaml, line 15 at r1 (raw file):

Previously, pigmej (Jędrzej Nowak) wrote…

can we have info what PW is that in a comment?

Done.


examples/ubuntu-vm-with-testuser.yaml, line 17 at r1 (raw file):

Previously, pigmej (Jędrzej Nowak) wrote…

can we have plain text password as comment there?

Done.


Comments from Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Jan 17, 2018

Review status: 0 of 10 files reviewed at latest revision, 6 unresolved discussions.


examples/fedora-vm-with-testuser.yaml, line 15 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Done.

Please see the note about 'testuser' for ubuntu below, also applies here


examples/fedora-vm-with-testuser.yaml, line 15 at r2 (raw file):

        groups: users
        lock_passwd: false
        # under this hash is encoded "testuser" as password

the password is "testuser"


examples/fedora-vm-with-testuser.yaml, line 19 at r2 (raw file):

        sudo: ALL=(ALL) NOPASSWD:ALL
    VirtletSSHKeys: |
      ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCaJEcFDXEK2ZbX0ZLS1EIYFZRbDAcRfuVjpstSc0De8+sV1aiu+dePxdkuDRwqFtCyk6dEZkssjOkBXtri00MECLkir6FcH3kKOJtbJ6vy3uaJc9w1ERo+wyl6SkAh/+JTJkp7QRXj8oylW5E20LsbnA/dIwWzAF51PPwF7A7FtNg9DnwPqMkxFo1Th/buOMKbP5ZA1mmNNtmzbMpMfJATvVyiv3ccsSJKOiyQr6UG+j7sc/7jMVz5Xk34Vd0l8GwcB0334MchHckmqDB142h/NCWTr8oLakDNvkfC1YneAfAO41hDkUbxPtVBG5M/o7P4fxoqiHEX+ZLfRxDtHB53 me@localhost

see the note for ubuntu 'testuser'


examples/ubuntu-vm-with-testuser.yaml, line 17 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

Done.

ubuntu example used to have problem of ssh key being ignored because of this 'testuser', so it's only possible to ssh into it using password. It would be better specify ssh-authorized-keys for testuser to make it possible to ssh into the VM using the example key, but still keeping the ability to log in with password which is handy for use with kubectl attach (e.g. to debug network issues that prevent ssh from working). Also, 'testuser' IIRC has some broken shell defaults unlike 'ubuntu' (maybe it's using /bin/sh or something)


examples/ubuntu-vm-with-testuser.yaml, line 17 at r2 (raw file):

        groups: users
        lock_passwd: false
        # under this hash is encoded "testuser" as password

the password is "testuser"


Comments from Reviewable

@jellonek
Copy link
Contributor Author

Reviewed 1 of 6 files at r2.
Review status: 1 of 10 files reviewed at latest revision, 6 unresolved discussions.


examples/ubuntu-vm-with-testuser.yaml, line 17 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

ubuntu example used to have problem of ssh key being ignored because of this 'testuser', so it's only possible to ssh into it using password. It would be better specify ssh-authorized-keys for testuser to make it possible to ssh into the VM using the example key, but still keeping the ability to log in with password which is handy for use with kubectl attach (e.g. to debug network issues that prevent ssh from working). Also, 'testuser' IIRC has some broken shell defaults unlike 'ubuntu' (maybe it's using /bin/sh or something)

In fedora - even if you will specify ssh key - after loging in as root with it you are asked to connect as other user, so that's probably similar to what was in ubuntu. I will add ssh-authorized-keys to testuser, and will try also check ssh-import-id, because that can be probably the reason why there is no way to login on testuser with keys.
For me testuser is mostly usable in broken network case, precisely to login using attach. But as it has no preset shell - it defaults to /bin/sh (in /etc/passwd shell part is empty).


Comments from Reviewable

@jellonek
Copy link
Contributor Author

Review status: 0 of 10 files reviewed at latest revision, 6 unresolved discussions.


examples/fedora-vm-with-testuser.yaml, line 15 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Please see the note about 'testuser' for ubuntu below, also applies here

Done.


examples/fedora-vm-with-testuser.yaml, line 15 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

the password is "testuser"

Done.


examples/fedora-vm-with-testuser.yaml, line 19 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

see the note for ubuntu 'testuser'

Done.


examples/ubuntu-vm-with-testuser.yaml, line 17 at r1 (raw file):

Previously, jellonek (Piotr Skamruk) wrote…

In fedora - even if you will specify ssh key - after loging in as root with it you are asked to connect as other user, so that's probably similar to what was in ubuntu. I will add ssh-authorized-keys to testuser, and will try also check ssh-import-id, because that can be probably the reason why there is no way to login on testuser with keys.
For me testuser is mostly usable in broken network case, precisely to login using attach. But as it has no preset shell - it defaults to /bin/sh (in /etc/passwd shell part is empty).

Ok, after setting ssh-authorized-keys + shell - everything looks fine now.


Comments from Reviewable

@jellonek
Copy link
Contributor Author

Review status: 0 of 10 files reviewed at latest revision, 6 unresolved discussions.


examples/ubuntu-vm-with-testuser.yaml, line 17 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

the password is "testuser"

Done.


Comments from Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Jan 17, 2018

:lgtm:


Review status: 0 of 10 files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@pigmej
Copy link
Contributor

pigmej commented Jan 17, 2018

:lgtm:


Reviewed 4 of 8 files at r1, 6 of 6 files at r2.
Review status: 8 of 10 files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@pigmej pigmej merged commit fa3a107 into master Jan 17, 2018
@pigmej pigmej deleted the jell/examples_cleanup branch January 17, 2018 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants