8000 ref(scheduler): clean up scheduler with better organisation by helgi · Pull Request #172 · deis/controller · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

ref(scheduler): clean up scheduler with better organisation #172

Merged
helgi merged 2 commits into from
Jan 5, 2016
Merged

ref(scheduler): clean up scheduler with better organisation #172

helgi merged 2 commits into from
Jan 5, 2016

Conversation

@helgi
8000
Copy link
Contributor
@helgi helgi commented Dec 21, 2015

This doesn't touch logic, only moves things around within the k8s.py in the first commit and then in turn makes it the __init__

It's a very simplified version of #48 - again, no code modifications here. Just sets the stage for more refactoring down the line

@helgi helgi self-assigned this Dec 21, 2015
@helgi helgi added this to the v2.0-beta1 milestone Dec 21, 2015
@helgi helgi force-pushed the cleaner_scheduler branch 3 times, most recently from 502d9f7 to 69ec089 Compare December 22, 2015 22:58
@helgi helgi force-pushed the cleaner_scheduler branch 3 times, most recently from c12a2e8 to 99e650c Compare December 30, 2015 23:08
arschles
arschles reviewed Dec 30, 2015
View reviewed changes
rootfs/scheduler/__init__.py
Copy link
Member
@arschles arschles Dec 30, 2015

Choose a reason for hiding this comment

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

issue for this? not blocking this PR

Copy link
Contributor Author
@helgi helgi Dec 30, 2015

Choose a reason for hiding this comment

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

There is no issue for this PR

Copy link
Member
@arschles arschles Dec 30, 2015

Choose a reason for hiding this comment

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

no, I mean an issue for the intermittent DNS errors, so we remember

Copy link
Contributor Author
@helgi helgi Dec 30, 2015

Choose a reason for hiding this comment

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

@mboersma added that in via #73 but I don't see an issue attached to that one

Copy link
Member
@arschles arschles Dec 30, 2015

Choose a reason for hiding this comment

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

Ok, seems like it's a well-known enough issue. Thanks

@arschles arschles added the LGTM1 label Dec 30, 2015
@arschles
Copy link
Member
arschles commented Dec 30, 2015

Do you need me to test this?

@helgi
Copy link
Contributor Author
helgi commented Dec 30, 2015

I'd love if you could throw it on your cluster and run the integration tests against a real k8s cluster since we have no k8s tests, barely any mocking for it even.

@arschles arschles mentioned this pull request Dec 30, 2015
Closed
@arschles
Copy link
Member
arschles commented Dec 30, 2015

@helgi ok, will test shortly

@arschles
Copy link
Member
arschles commented Dec 30, 2015

@helgi I built and pushed my own image with make docker-build docker-push and replaced the existing workflow RC with mine in a stock helm cluster. Seems to fail to come up:

ENG000656:example-go aaronschlesinger$ kd logs -f deis-workflow-zsg3a
Syncing...
Creating tables ...
Installing custom SQL ...
Installing indexes ...
Installed 0 object(s) from 0 fixture(s)
Migrating...
Running migrations for authtoken:
- Nothing to migrate.
 - Loading initial data for authtoken.
Installed 0 object(s) from 0 fixture(s)
Running migrations for api:
 - Migrating forwards to 0026_auto__chg_field_config_tags__chg_field_config_values__chg_field_config.
 > api:0001_initial
Traceback (most recent call last):
  File "./manage.py", line 13, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/lib/python2.7/site-packages/django/core/management/__init__.py", line 399, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python2.7/site-packages/django/core/management/__init__.py", line 392, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/lib/python2.7/site-packages/django/core/management/base.py", line 242, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/lib/python2.7/site-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python2.7/site-packages/django/core/management/base.py", line 415, in handle
    return self.handle_noargs(**options)
  File "/usr/lib/python2.7/site-packages/south/management/commands/syncdb.py", line 103, in handle_noargs
    management.call_command('migrate', **options)
  File "/usr/lib/python2.7/site-packages/django/core/management/__init__.py", line 159, in call_command
    return klass.execute(*args, **defaults)
  File "/usr/lib/python2.7/site-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python2.7/site-packages/south/management/commands/migrate.py", line 111, in handle
    ignore_ghosts = ignore_ghosts,
  File "/usr/lib/python2.7/site-packages/south/migration/__init__.py", line 220, in migrate_app
    success = migrator.migrate_many(target, workplan, database)
  File "/usr/lib/python2.7/site-packages/south/migration/migrators.py", line 256, in migrate_many
    result = migrator.__class__.migrate_many(migrator, target, migrations, database)
  File "/usr/lib/python2.7/site-packages/south/migration/migrators.py", line 331, in migrate_many
    result = self.migrate(migration, database)
  File "/usr/lib/python2.7/site-packages/south/migration/migrators.py", line 133, in migrate
    result = self.run(migration, database)
  File "/usr/lib/python2.7/site-packages/south/migration/migrators.py", line 106, in run
    south.db.db.current_orm = self.orm(migration)
  File "/usr/lib/python2.7/site-packages/south/migration/migrators.py", line 281, in orm
    return migration.orm()
  File "/usr/lib/python2.7/site-packages/south/utils/__init__.py", line 62, in method
    value = function(self)
  File "/usr/lib/python2.7/site-packages/south/migration/base.py", line 443, in orm
    return FakeORM(self.migration_class(), self.app_label())
  File "/usr/lib/python2.7/site-packages/south/orm.py", line 48, in FakeORM
    _orm_cache[args] = _FakeORM(*args)  
  File "/usr/lib/python2.7/site-packages/south/orm.py", line 127, in __init__
    self.models[name] = self.make_model(app_label, model_name, data)
  File "/usr/lib/python2.7/site-packages/south/orm.py", line 324, in make_model
    field = self.eval_in_context(code, app, extra_imports)
  File "/usr/lib/python2.7/site-packages/south/orm.py", line 235, in eval_in_context
    raise ValueError("Cannot import the required field '%s'" % value)
ValueError: Cannot import the required field 'json_field.fields.JSONField'

@helgi
Copy link
Contributor Author
helgi commented Dec 31, 2015

Ah hmm @slack ran another PR and didn't see this problem. Seems like the djangos didn't like the removal of the other json_fields package.

Hmm I can either reintroduce it in a sep PR or we can land #90 and it won't be a problem anymore because the south migrations will be squashed into a singular native django migration format

@arschles
Copy link
Member
arschles commented Dec 31, 2015

@helgi I'm 👍 on landing #90. Just reviewed that and LGTM2-ed it. Can you let me know when you've rebased this with the contents of #90 and I can retest this?

@arschles arschles closed this Dec 31, 2015
@helgi helgi reopened this Dec 31, 2015
@helgi helgi force-pushed the cleaner_scheduler branch from 99e650c to d00f8f1 Compare December 31, 2015 00:21
@arschles
Copy link
Member
arschles commented Dec 31, 2015

@helgi things look better now. Output below:

Workflow startup logs:

ENG000656:workflow aaronschlesinger$ kd logs -f deis-workflow-pxwt0
Operations to perform:
  Synchronize unmigrated apps: guardian, jsonfield, corsheaders, registry, rest_framework
  Apply all migrations: authtoken, sess
8000
ions, admin, sites, auth, contenttypes, api
Synchronizing apps without migrations:
  Creating tables...
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying contenttypes.0001_initial... FAKED
  Applying auth.0001_initial... FAKED
  Applying admin.0001_initial... FAKED
  Applying api.0001_initial... OK
  Applying authtoken.0001_initial... FAKED
  Applying sessions.0001_initial... FAKED
  Applying sites.0001_initial... FAKED
[2015-12-31 00:41:03 +0000] [116] [INFO] Starting gunicorn 19.3.0
[2015-12-31 00:41:03 +0000] [116] [INFO] Listening at: http://0.0.0.0:8000 (116)
[2015-12-31 00:41:03 +0000] [116] [INFO] Using worker: sync
[2015-12-31 00:41:03 +0000] [120] [INFO] Booting worker with pid: 120
[2015-12-31 00:41:03 +0000] [124] [INFO] Booting worker with pid: 124
[2015-12-31 00:41:03 +0000] [125] [INFO] Booting worker with pid: 125
Publishing DB state to etcd...
Done Publishing DB state to etcd.
deis-controller running...

Zero to git push:

ENG000656:example-go aaronschlesinger$ deis register deis.108.59.84.244.xip.io
username: arschles
password: 
password (confirm): 
email: arschles@gmail.com
Registered arschles
Logged in as arschles
ENG000656:example-go aaronschlesinger$ deis keys:add ~/.ssh/id_rsa.pub 
Uploading id_rsa.pub to deis... done
ENG000656:example-go aaronschlesinger$ deis create gotest
Creating Application... done, created gotest
Git remote deis added
remote available at ssh://git@deis.108.59.84.244.xip.io:2222/gotest.git
ENG000656:example-go aaronschlesinger$ git push deis master
The authenticity of host '[deis.108.59.84.244.xip.io]:2222 ([108.59.84.244]:2222)' can't be established.
ECDSA key fingerprint is SHA256:0QSJc8AT8qKofdmh8vKJoIYgkk9Gv/tM/JXZkEAPFsk.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '[deis.108.59.84.244.xip.io]:2222,[108.59.84.244]:2222' (ECDSA) to the list of known hosts.
Counting objects: 270, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (125/125), done.
Writing objects: 100% (270/270), 3.02 MiB | 732.00 KiB/s, done.
Total 270 (delta 148), reused 241 (delta 131)
-----> Starting build
-----> Go app detected
-----> Checking Godeps/Godeps.json file.
-----> Installing go1.4.2... done
-----> Running: godep go install -tags heroku ./...
-----> Discovering process types
       Procfile declares types -> web
-----> Compiled slug size is 4.7M
-----> Build complete.
-----> Launching app.
-----> Launching... 
       done, gotest:v1 deployed to Deis

       http://gotest.localhost

       To learn more, use `deis help` or visit http://deis.io

To ssh://git@deis.108.59.84.244.xip.io:2222/gotest.git
 * [new branch]      master -> master
ENG000656:example-go aaronschlesinger$ curl gotest.108.59.84.244.xip.io
Powered by Deis
Release v2 on gotest-v2-web-v1un0

Scale didn't work, where it did before:

ENG000656:example-go aaronschlesinger$ deis releases -a gotest
=== gotest Releases
v1  2015-12-31T00:45:07UTC  arschles created initial release
ENG000656:example-go aaronschlesinger$ deis scale web=4 -a gotest
Scaling processes... but first, coffee!
Error: 
400 BAD REQUEST
detail: No build associated with this release

@helgi helgi force-pushed the cleaner_scheduler branch from d00f8f1 to 609bb8e Compare December 31, 2015 19:15
@helgi
Copy link
Contributor Author
helgi commented Jan 1, 2016

@arschles I did some fixes in there but I haven't had time to properly test it myself

@helgi helgi force-pushed the cleaner_scheduler branch from 609bb8e to acc4b0c Compare January 5, 2016 00:05
@arschles
Copy link
Member
arschles commented Jan 5, 2016

@helgi did some testing with this. TL;DR is LGTM

Longer explanation: it starts up fine from an empty DB (migrations don't seem to work, but that's expected afaik), and registering, adding keys and git push all work.

However, the app doesn't seem to be running after the git push. deis info says the app is up and a domain exists for it of the expected name, but the app is inaccessible via curl.

Upon talking with you and @krancour I understand this bug may be happening because router and workflow don't agree on the annotations used for domain handling. Is that issue tracked somewhere (perhaps here?) Logs below.

Startup:

ENG000656:example-go aaronschlesinger$ kd logs -f deis-workflow-3uhat
system information:
Django Version: 1.9
Python 2.7.10
Operations to perform:
  Apply all migrations: authtoken, sessions, admin, guardian, sites, auth, contenttypes, api
Running migrations:
  Rendering model states... DONE
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying api.0001_initial... OK
  Applying api.0002_auto_20151215_0352... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying authtoken.0001_initial... OK
  Applying guardian.0001_initial... OK
  Applying sessions.0001_initial... OK
  Applying sites.0001_initial... OK
  Applying sites.0002_alter_domain_unique... OK
[2016-01-05 00:27:25 +0000] [132] [INFO] Starting gunicorn 19.4.1
[2016-01-05 00:27:25 +0000] [132] [INFO] Listening at: http://0.0.0.0:8000 (132)
[2016-01-05 00:27:25 +0000] [132] [INFO] Using worker: sync
[2016-01-05 00:27:25 +0000] [139] [INFO] Booting worker with pid: 139
[2016-01-05 00:27:25 +0000] [140] [INFO] Booting worker with pid: 140
[2016-01-05 00:27:25 +0000] [141] [INFO] Booting worker with pid: 141
Publishing DB state to etcd...
Done Publishing DB state to etcd.
deis-controller running...

Registering, saving key, pushing:

ENG000656:example-go aaronschlesinger$ deis register deis.108.59.84.244.xip.io
username: arschles
password: 
password (confirm): 
email: arschles@gmail.com
Registered arschles
Logged in as arschles
ENG000656:example-go aaronschlesinger$ deis apps
=== Apps
ENG000656:example-go aaronschlesinger$ deis keys:add ~/.ssh/id_rsa.pub 
Uploading id_rsa.pub to deis... done
ENG000656:example-go aaronschlesinger$ deis create gotest
Creating Application... done, created gotest
Git remote deis added
remote available at ssh://git@deis.108.59.84.244.xip.io:2222/gotest.git
ENG000656:example-go aaronschlesinger$ git push deis master
The authenticity of host '[deis.108.59.84.244.xip.io]:2222 ([108.59.84.244]:2222)' can't be established.
ECDSA key fingerprint is SHA256:LU+BUQcukbLu72ab+Q8hgTCfFzKYgUk2R7S5pJq+scg.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '[deis.108.59.84.244.xip.io]:2222,[108.59.84.244]:2222' (ECDSA) to the list of known hosts.
Counting objects: 273, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (128/128), done.
Writing objects: 100% (273/273), 3.02 MiB | 669.00 KiB/s, done.
Total 273 (delta 151), reused 241 (delta 131)
-----> Starting build
-----> Go app detected
-----> Checking Godeps/Godeps.json file.
-----> Installing go1.4.2... done
-----> Running: godep go install -tags heroku ./...
-----> Discovering process types
       Procfile declares types -> web
-----> Compiled slug size is 4.7M
-----> Build complete.
-----> Launching app.
-----> Launching... 
       done, gotest:v2 deployed to Deis

       http://gotest.localhost

       To learn more, use `deis help` or visit http://deis.io

To ssh://git@deis.108.59.84.244.xip.io:2222/gotest.git
 * [new branch]      master -> master
ENG000656:example-go aaronschlesinger$ deis info
=== gotest Application
updated:  2016-01-05T00:29:55UTC
uuid:     05325e2e-58be-49b3-b2d4-8df7bbc4de42
created:  2016-01-05T00:28:43UTC
url:      gotest.localhost
owner:    arschles
id:       gotest

=== gotest Processes
--- web:
web.1 up (v3)

=== gotest Domains
gotest

@arschles arschles added the LGTM2 label Jan 5, 2016
@arschles
Copy link
Member
arschles commented Jan 5, 2016

also @helgi can you remove the in progress label before you merge?

@helgi helgi removed the in progress label Jan 5, 2016
@krancour
Copy link
Contributor
krancour commented Jan 5, 2016

Is that issue tracked somewhere?

No... it was a transient issue that occurred because there was a router PR and a workflow PR that were complimentary and should have been merged in tandem, but were not.

@arschles arschles removed the LGTM2 label Jan 5, 2016
@slack
Copy link
Member
slack commented Jan 5, 2016

Ah ok. Ran into the router issue...

Created, scaled up/down, created config... Looks ok

@slack slack added the LGTM2 label Jan 5, 2016
helgi added a commit that referenced this pull request Jan 5, 2016
@helgi
Merge pull request #172 from helgi/cleaner_scheduler
d7e3317 
ref(scheduler): clean up scheduler with better organisation
@helgi helgi merged commit d7e3317 into deis:master Jan 5, 2016
@helgi helgi deleted the cleaner_scheduler branch January 5, 2016 01:20
@arschles
Copy link
Member
arschles commented Jan 5, 2016

@krancour I see, makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@helgi helgi

Labels

k8s LGTM1 LGTM2 needs manual testing

Projects

None yet

Milestone

v2.0-beta1

Development

Successfully merging this pull request may close these issues.

4 participants

@helgi @arschles @krancour @slack
0